(feat): io submodule
#1682
(feat): io submodule
#1682
Conversation
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
ilan-gold
added
skip-gpu-ci
topic: io
breaking change ‼️
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1682 +/- ##
==========================================
- Coverage 86.87% 84.50% -2.38%
==========================================
Files 39 40 +1
Lines 6036 6047 +11
==========================================
- Hits 5244 5110 -134
- Misses 792 937 +145
Flags with carried forward coverage won't be shown. Click here to find out more.
|
src/anndata/_io/__init__.py
Outdated
| # We use this in test by attribute access | ||
| from . import specs # noqa: F401, E402 |
There was a problem hiding this comment.
Any reason why we shouldn’t just stop doing that?
There was a problem hiding this comment.
Would need to investigate what it's doing there or what that comment even means. Maybe an issue?
There was a problem hiding this comment.
Well, I deleted it and nothing happened so....
There was a problem hiding this comment.
I put that import and comment there because Isaac made the tests access this by attribute:
import anndata as ad
do_stuff_with(ad._io.specs.foobar)instead of importing that submodule. It happened to work because some other code somewhere else caused anndata._io.specs to be imported, but “happens to work because of side effects” is not what I want to build upon.
Now that the tests no longer access stuff that way, this import is no longer necessary.
|
I got rid of a few more obsolete import hacks: a947d3e Why
breaking change ‼️
|
Hmm I guess I was just thinking "begins process for breaking change" but yea, it's not. |
ilan-gold
removed
the
breaking change ‼️
I don't think they were so obsolete: https://dev.azure.com/scverse/anndata/_build/results?buildId=8163&view=logs&j=60c6783d-556f-53a9-3d94-3b9ccc51ad8d&t=3522b173-13e1-56b6-93d8-ed20bad6f9bb&l=60! I cleaned it up a bit to be more uniform. |
|
Ah weird, I’m sure I ran the tests without zarr locally, but looking at the code I guess I probably didn’t. I don’t like this:
edit: fixed in 2afc94a |
Co-authored-by: Isaac Virshup <[email protected]>
Co-authored-by: Philipp A. <[email protected]>
A brief summary of how this was handled and why:
iorelated to i/oread_zarrandread_h5adat the top-level for now, although I think that should be deprecated in favor of everything going throughioin the future despite their "high" status above the others. They are now documented as being fromioto begin to encourage that behavior although we aren't throwing warnings for e.g.,anndata.read_h5adyetfrom anndata._io import blahsince uhhhhhhh yikes by leaving that module alone. However, at least if anyone tries to import from the top-levelfrom anndata._io import read_h5ad, it will warn. Also, I added a public note about this issue since it seems pervasive.pytest.warnscan't run reliably every time because the warning doesn't come every time....not sure what's up with thatiosubmodule #1681