zarr-developers
Make methods of abstract base class async
Fixes #2377.
See https://github.com/microsoft/pyright/discussions/4741.
TODO:
- [ ] Add unit tests and/or doctests in docstrings
- [ ] Add docstrings and API docs for any new/modified user-facing classes and functions
- [ ] New/modified features documented in docs/tutorial.rst
- [ ] Changes documented in docs/release.rst
- [x] GitHub Actions have all passed
- [x] Test coverage is 100% (Codecov passes)
I think a better type for these functions would be:
async def list(self) -> AsyncIterator[str]:
Most implementations probably won't need these functions to be async, but it's a more general type for some implementation that may need it.
I think a better type for these functions would be:
async def list(self) -> AsyncIterator[str]:
The roadmap defines the Store API as follows and should be updated:
async def list(self) -> List[str]:
... # required for listable stores
async def list_prefix(self, prefix: str) -> List[str]:
... # required for listable stores
async def list_dir(self, prefix: str) -> List[str]:
... # required for listable stores
~~All current child classes use the prefix argument. I don't think it should be removed.~~
As for the return value, not sure why it changed from List[str] to AsyncGenerator[str, None] in #1844. Switch back to AsyncGenerator[str]?
~All current child classes use the
prefixargument. I don't think it should be removed.~
Sorry, I just missed the argument in the example, definitely we shouldn't drop it. My point was about returning AsyncIterator instead of AsyncGenerator, the later is more of an implementation detail. We definitely don't want list because that would force the whole thing to live in memory at once.
Meanwhile, I tried to revert the return type from AsyncGenerator[str, None] to AsyncGenerator[str], to see what happens.
As for the AsyncGenerator → AsyncIterator change, how about addressing it in an issue and a separate PR?
As for the AsyncGenerator → AsyncIterator change, how about addressing it in an issue and a separate PR?
It's definitely not a big deal, AsyncGenerator[str] also works. But, doing it in separate PR would require changing the abc and every implementation twice.
OK, let me try then. It's just that's I'll have to modify all child classes.
I tried briefly, but I cannot get typing to work after rebasing. I need to take a thorough look.