Skip to content

Comments

Final review/0.0.3#111

Merged
MeyerBender merged 6 commits intomainfrom
final_review/0.0.3
Feb 20, 2026
Merged

Final review/0.0.3#111
MeyerBender merged 6 commits intomainfrom
final_review/0.0.3

Conversation

@LazDaria
Copy link
Owner

@LazDaria LazDaria commented Feb 18, 2026

In the tests for the supervised module, markers_from_reference results in zero markers due to the small scRNA-seq subset. So tests are currently not doing anything. Once we have a larger scRNA-seq test dataset uploaded (that actually includes reference markers), we can uncomment test_mutually_exclusive_coexpression_rate.py .

@LazDaria LazDaria requested a review from MeyerBender February 18, 2026 17:06
- "fraction_heterotypic_overlap": pd.DataFrame
- "vertical_signal_integrity_per_cell": pd.DataFrame (only if vsi_map is not None)
"""
assert self.points_z_key is not None, (
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would put this directly into vl.similarity_top_bottom to avoid asserts in the SegTraQ class

Copy link
Collaborator

Choose a reason for hiding this comment

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

need to still remove this


> ⚠️ Note: SegTraQ is under active development.
> Features, interfaces, and functionality may change in upcoming releases.
> Features, interfaces, and functionality may change in upcoming releases.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you also want to add this to docs/index?

Copy link
Owner Author

Choose a reason for hiding this comment

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

I think it is enough to have it in the README for now.

Copy link
Collaborator

@mjemons mjemons left a comment

Choose a reason for hiding this comment

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

just left one comment, else LGTM



def test_neighbor_contamination_realdata_inplace_outputs(sdata_labeled, adata_ref):
markers = st.markers_from_reference(adata_ref.copy(), cell_type_key="celltype_major")
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe we can change the default thresholds a bit such that we have enough cells for the tests?

@MeyerBender
Copy link
Collaborator

While we're at it, we should have a better warning for if nucleus_boundaries aren't available in the constructor.
Something like "Set nucleus_shapes_key to None if you don't have this."

@MeyerBender
Copy link
Collaborator

Also: deprecated warning:

AssertionError: Cannot compute IoUs: `nucleus_shapes_key` is None. Define a valid nucleus shape layer in the `SegTraQ` constructor before running `nc` metrics.

This should not be nc anymore.

@MeyerBender
Copy link
Collaborator

We also have a ValueError in cs: Neighbors not found in adata. Please compute neighbors first by running sc.pp.neighbors(adata). Should make this a bit clearer (how to access the adata object).

@mjemons mjemons self-requested a review February 20, 2026 09:55
- "fraction_heterotypic_overlap": pd.DataFrame
- "vertical_signal_integrity_per_cell": pd.DataFrame (only if vsi_map is not None)
"""
assert self.points_z_key is not None, (
Copy link
Collaborator

Choose a reason for hiding this comment

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

need to still remove this

@MeyerBender MeyerBender merged commit c1aeafb into main Feb 20, 2026
4 checks passed
@MeyerBender MeyerBender deleted the final_review/0.0.3 branch February 20, 2026 10:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants