document zarr functions properly

[flying-sheep]

No description provided.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.50%. Comparing base (045d7c9) to head (1a11b47).
Report is 1 commits behind head on ig/io_module.

Additional details and impacted files

@@               Coverage Diff                @@
##           ig/io_module    #1688      +/-   ##
================================================
- Coverage         84.50%   84.50%   -0.01%     
================================================
  Files                40       40              
  Lines              6049     6047       -2     
================================================
- Hits               5112     5110       -2     
  Misses              937      937              

Flag	Coverage Δ
	84.50% <100.00%> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
src/anndata/io.py	100.00% <100.00%> (ø)

[flying-sheep]

I just noticed that the reson we don’t do that is probably zarr’s slow import.

[flying-sheep]

OK, I pushed an alternative that works. Another one would be

def __getattr__(attr_name: Literal["read_zarr", "write_zarr"]):
    if attr_name in {"read_zarr", "write_zarr"}:
        from ._io import zarr

        return getattr(zarr, attr_name)

    raise AttributeError(attr_name)


__all__ = [
    ...
    "read_zarr",  # noqa: F822
    ...
    "write_zarr",  # noqa: F822
    ...
]

which is less hacky, but makes type checking sadder.

[ilan-gold]

OK, I pushed an alternative that works. Another one would be

In any case, I think this would probably be better. In vscode, this would also give the docstring as well, no?

[flying-sheep]

In any case, I think this would probably be better.

Hmm, the from .io import read_zarr in anndata/__init__.py would probably cause this attempt to avoid importing zarr to fail

In vscode, this would also give the docstring as well, no?

nope, PyRight doesn’t understand this, even when being super verbose and adding @overloads to __getattr__.

I feel like the hack is the only thing that works.

We could also move the hack into docs/conf.py or one of our custom extensions.

[ilan-gold]

We could also move the hack into docs/conf.py or one of our custom extensions.

I'm ok with how things are right now, but if it's clean and easy to do this, wouldn't be so bad. It should be straightforward to override the doc/import location right? We have something for this?

[flying-sheep]

The stuff we have for overriding locations is only for links. In this case we want autosummary to see the real function, so we need to replace the shim with it.

An altenative I can think of: I could replace the condition with if 'zarr' in sys.modules, then if people import zarr first, the shims get removed.
We could then do just that in conf.py and add a comment documenting this.

[ilan-gold]

We could then do just that in conf.py and add a comment documenting this.

I think that's reasonable

[flying-sheep]

OK, there we go. This should work nicely, for static typing, docs, and at runtime