Sliding window improvements #931
Conversation
… to minimize the partial uncovered area - aligned_min_x and aligned_min_y added to _calculate_window_corners function to remove partial regions at the beginning of the slides - max_n_cells, n_splits, and split_line parameters added to the sliding_window function
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
src/squidpy/tl/_sliding_window.py
Outdated
| %(library_key)s | ||
| coord_columns: Tuple[str, str] | ||
| Tuple of column names in `adata.obs` that specify the coordinates (x, y), e.i. ('globalX', 'globalY') | ||
| window_size: int |
There was a problem hiding this comment.
Please adjust the description: include option of tuple
There was a problem hiding this comment.
Overall there are some function parameters that are not described in the doctoring. Please make sure to include documentation for all.
| if isinstance(adata, SpatialData): | ||
| adata = adata.table | ||
|
|
||
| assert max_n_cells is None or n_splits is None, ( |
There was a problem hiding this comment.
Also: assert that not all window_size, max_n_cells and n_splits are None.
FrancescaDr
left a comment
FrancescaDr
left a comment
There was a problem hiding this comment.
Hi @mossishahi, thanks for the implementation. Please don't forget to add tests for all the functions that you added. I also recommend to check out the squidpy contribution guidelines to make sure the code and docstrings are correctly formatted.
src/squidpy/tl/_sliding_window.py
Outdated
| raise ValueError(f"Library key '{library_key}' not found in adata.obs") | ||
|
|
||
| libraries = [None] if library_key is None else adata.obs[library_key].unique() | ||
| if library_key is None and "fov" not in adata.obs.columns: |
There was a problem hiding this comment.
Maybe call it temp_fov for temporal_fov and remove it at the end of the code.
src/squidpy/tl/_sliding_window.py
Outdated
| window_sizes = [] | ||
|
|
||
| if window_size is None: | ||
| if max_n_cells is None and n_splits is None: |
There was a problem hiding this comment.
Earlier you added a check that this case does not exists, so I suggest to remove this.
| n_splits=None, | ||
| drop_partial_windows: bool = False, | ||
| square: bool = False, | ||
| window_size_per_library_key: str = "equal", |
There was a problem hiding this comment.
Add explanation to code string.
| sliding_window_df.loc[obs_indices, col_name] = True | ||
| sliding_window_df.loc[:, col_name].fillna(False, inplace=True) | ||
|
|
||
| if overlap == 0: |
There was a problem hiding this comment.
Also enable drop_partial_windows for the case of no overlap.
|
|
||
|
|
||
| def _optimize_tile_size(L, W, A_min=None, A_max=None, square=False, split_line="v"): | ||
| """ |
There was a problem hiding this comment.
Adjust doctoring to squidpy/scverse style guide.
3 participants
IMPORTANT: Please search among the Pull requests before creating one.
Description
How has this been tested?
Closes