-
Notifications
You must be signed in to change notification settings - Fork 25
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
[python] Add unit tests for obsm, obsp, and to_anndata #1934
Conversation
query.obsm("baz").tables().concat() | ||
== soma_experiment.ms["RNA"] | ||
.obsm["baz"] | ||
.read((obs_slice, range(50))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
magic number 50 is in a few places. Would be good to at least use a constant variable (e.g., N_FEAT or some such)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, let me add it to a constant.
Codecov Report
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #1934 +/- ##
===========================================
+ Coverage 63.11% 89.74% +26.62%
===========================================
Files 106 34 -72
Lines 10056 3598 -6458
===========================================
- Hits 6347 3229 -3118
+ Misses 3709 369 -3340
Flags with carried forward coverage won't be shown. Click here to find out more.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one minor nit in a comment - otherwise LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! One thing I would recommend to make dev/review a bit easier is to substitute in the development revision into the setup.py file for the duration of the review, so it can be more easily evaluated during the review process, but once the upstream is released, you can cut it over to the final version and fully integrate everything.
Issue and/or context:
Resolves #1930 together with single-cell-data/SOMA#179. Requires the latter to be released before tests can pass, so this is still marked as draft.
Changes:
This PR only contains the unit tests. All the code changes are in the SOMA repo.
Notes for Reviewer: