Skip to content
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.

Sign up for GitHub

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

Fix cell_transition bug #751

Merged
merged 5 commits into from
Oct 10, 2024
Merged

Fix cell_transition bug #751

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
42550e3
fix the bug and set observed=False for future pandas and warning
selmanozleyen Oct 7, 2024
eb431e2
Merge branch 'main' into fix/celltype-mapping
selmanozleyen Oct 8, 2024
8f70c1e
update _cell_transition_online
selmanozleyen Oct 9, 2024
159eba0
refactor some functions
selmanozleyen Oct 9, 2024
92acfbe
remove some unnecesary lines
selmanozleyen Oct 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 13 additions & 8 deletions src/moscot/base/problems/_mixins.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,22 +198,24 @@ def _cell_transition_online(
)
df_source = _get_df_cell_transition(
self.adata,
[source_annotation_key] if aggregation_mode == "cell" else [source_annotation_key, target_annotation_key],
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why can we drop the distinction between the two aggregation modes?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since we always expected source_annotation_key=target_annotation_key this code worked. But since target_annotation_key might not be in self.adata, I am not sure why it was implemented this way. For example if it's not this way it would expect test_col_2 on adata_sp

[source_annotation_key],
key,
source,
)
df_target = _get_df_cell_transition(
self.adata if other_adata is None else other_adata,
[target_annotation_key] if aggregation_mode == "cell" else [source_annotation_key, target_annotation_key],
[target_annotation_key],
key if other_adata is None else other_key,
target,
)

df_source = df_source.rename(columns={source_annotation_key: "res_annotation"})
df_target = df_target.rename(columns={target_annotation_key: "res_annotation"})
res_annotation_key = "res_annotation"
source_annotations_verified, target_annotations_verified = _validate_annotations(
df_source=df_source,
df_target=df_target,
source_annotation_key=source_annotation_key,
target_annotation_key=target_annotation_key,
source_annotation_key=res_annotation_key,
target_annotation_key=res_annotation_key,
source_annotations=source_annotations,
target_annotations=target_annotations,
aggregation_mode=aggregation_mode,
Expand All @@ -236,6 +238,7 @@ def _cell_transition_online(
annotations_1=source_annotations_verified,
annotations_2=target_annotations_verified,
df=df_target,
df_key=res_annotation_key,
tm=tm,
forward=True,
)
Expand All @@ -247,6 +250,7 @@ def _cell_transition_online(
annotations_1=target_annotations_verified,
annotations_2=source_annotations_verified,
df=df_source,
df_key=res_annotation_key,
tm=tm,
forward=False,
)
Expand All @@ -256,7 +260,7 @@ def _cell_transition_online(
tm = self._cell_aggregation_transition( # type: ignore[attr-defined]
source=source,
target=target,
annotation_key=target_annotation_key,
annotation_key=res_annotation_key,
annotations_1=source_annotations_verified,
annotations_2=target_annotations_verified,
df_1=df_target,
Expand All @@ -269,7 +273,7 @@ def _cell_transition_online(
tm = self._cell_aggregation_transition( # type: ignore[attr-defined]
source=source,
target=target,
annotation_key=source_annotation_key,
annotation_key=res_annotation_key,
annotations_1=target_annotations_verified,
annotations_2=source_annotations_verified,
df_1=df_source,
Expand Down Expand Up @@ -483,6 +487,7 @@ def _annotation_aggregation_transition(
annotation_key: str,
annotations_1: list[Any],
annotations_2: list[Any],
df_key: str,
df: pd.DataFrame,
tm: pd.DataFrame,
forward: bool,
Expand All @@ -503,7 +508,7 @@ def _annotation_aggregation_transition(
key_added=None,
)
df["distribution"] = result
cell_dist = df[df[annotation_key].isin(annotations_2)].groupby(annotation_key).sum(numeric_only=True)
cell_dist = df[df[df_key].isin(annotations_2)].groupby(df_key, observed=False).sum(numeric_only=True)
cell_dist /= cell_dist.sum()
tm.loc[subset, :] = [
cell_dist.loc[annotation, "distribution"] if annotation in cell_dist.distribution.index else 0
Expand Down
Loading