-
Notifications
You must be signed in to change notification settings - Fork 71
Added parameter to sd.concatenate
to control whether to merge coordinate systems with the same name
#919
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
timtreis
wants to merge
10
commits into
main
Choose a base branch
from
bugfix/issue918-sdconcatenate-doesnt-apply-suffix-to-coordinate-systems
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Added parameter to sd.concatenate
to control whether to merge coordinate systems with the same name
#919
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
a78d460
added parameter to optionally merge cs
timtreis 19ef82a
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] 0cc33d9
fixed ruff
timtreis 604511e
added helper function to sanitize table objects
timtreis d542564
Revert "added helper function to sanitize table objects"
timtreis 2cffb17
Merge branch 'main' into bugfix/issue918-sdconcatenate-doesnt-apply-s…
timtreis 1a800c7
copilot feedback
timtreis 3f32484
merge
timtreis c99e304
code review with changes
LucaMarconato 5c5fb38
Merge branch 'main' into bugfix/issue918-sdconcatenate-doesnt-apply-s…
LucaMarconato File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,7 +13,12 @@ | |
|
||
from spatialdata._core._utils import _find_common_table_keys | ||
from spatialdata._core.spatialdata import SpatialData | ||
from spatialdata.models import SpatialElement, TableModel, get_table_keys | ||
from spatialdata.models import TableModel, get_table_keys | ||
from spatialdata.transformations import ( | ||
get_transformation, | ||
remove_transformation, | ||
set_transformation, | ||
) | ||
|
||
__all__ = [ | ||
"concatenate", | ||
|
@@ -81,7 +86,8 @@ def concatenate( | |
concatenate_tables: bool = False, | ||
obs_names_make_unique: bool = True, | ||
modify_tables_inplace: bool = False, | ||
attrs_merge: StrategiesLiteral | Callable[[list[dict[Any, Any]]], dict[Any, Any]] | None = None, | ||
merge_coordinate_systems_on_name: bool = False, | ||
attrs_merge: (StrategiesLiteral | Callable[[list[dict[Any, Any]]], dict[Any, Any]] | None) = None, | ||
**kwargs: Any, | ||
) -> SpatialData: | ||
""" | ||
|
@@ -110,6 +116,8 @@ def concatenate( | |
modify_tables_inplace | ||
Whether to modify the tables in place. If `True`, the tables will be modified in place. If `False`, the tables | ||
will be copied before modification. Copying is enabled by default but can be disabled for performance reasons. | ||
merge_coordinate_systems_on_name | ||
timtreis marked this conversation as resolved.
Show resolved
Hide resolved
|
||
Whether to keep coordinate system names unchanged (True) or add suffixes (False). | ||
attrs_merge | ||
How the elements of `.attrs` are selected. Uses the same set of strategies as the `uns_merge` argument of [anndata.concat](https://anndata.readthedocs.io/en/latest/generated/anndata.concat.html) | ||
kwargs | ||
|
@@ -141,6 +149,7 @@ def concatenate( | |
rename_tables=not concatenate_tables, | ||
rename_obs_names=obs_names_make_unique and concatenate_tables, | ||
modify_tables_inplace=modify_tables_inplace, | ||
merge_coordinate_systems_on_name=merge_coordinate_systems_on_name, | ||
) | ||
|
||
ERROR_STR = ( | ||
|
@@ -222,12 +231,27 @@ def _fix_ensure_unique_element_names( | |
rename_tables: bool, | ||
rename_obs_names: bool, | ||
modify_tables_inplace: bool, | ||
merge_coordinate_systems_on_name: bool, | ||
) -> list[SpatialData]: | ||
elements_by_sdata: list[dict[str, SpatialElement]] = [] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this refactoring is fine |
||
tables_by_sdata: list[dict[str, AnnData]] = [] | ||
sdatas_fixed = [] | ||
for suffix, sdata in sdatas.items(): | ||
elements = {f"{name}-{suffix}": el for _, name, el in sdata.gen_spatial_elements()} | ||
elements_by_sdata.append(elements) | ||
# Create new elements dictionary with suffixed names | ||
elements = {} | ||
for _, name, el in sdata.gen_spatial_elements(): | ||
new_element_name = f"{name}-{suffix}" | ||
if not merge_coordinate_systems_on_name: | ||
# Set new transformations with suffixed coordinate system names | ||
transformations = get_transformation(el, get_all=True) | ||
assert isinstance(transformations, dict) | ||
|
||
remove_transformation(el, remove_all=True) | ||
for cs, t in transformations.items(): | ||
new_cs = f"{cs}-{suffix}" | ||
set_transformation(el, t, to_coordinate_system=new_cs) | ||
|
||
elements[new_element_name] = el | ||
|
||
# Handle tables with suffix | ||
tables = {} | ||
for name, table in sdata.tables.items(): | ||
if not modify_tables_inplace: | ||
|
@@ -251,9 +275,8 @@ def _fix_ensure_unique_element_names( | |
# fix the table name | ||
new_name = f"{name}-{suffix}" if rename_tables else name | ||
tables[new_name] = table | ||
tables_by_sdata.append(tables) | ||
sdatas_fixed = [] | ||
for elements, tables in zip(elements_by_sdata, tables_by_sdata, strict=True): | ||
sdata = SpatialData.init_from_elements(elements, tables=tables) | ||
sdatas_fixed.append(sdata) | ||
|
||
# Create new SpatialData object with suffixed elements and tables | ||
sdata_fixed = SpatialData.init_from_elements(elements, tables=tables) | ||
sdatas_fixed.append(sdata_fixed) | ||
return sdatas_fixed |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.