Skip to content
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

Inconsistent overridden method #2377

Open
DimitriPapadopoulos opened this issue Oct 15, 2024 · 3 comments · May be fixed by #2381
Open

Inconsistent overridden method #2377

DimitriPapadopoulos opened this issue Oct 15, 2024 · 3 comments · May be fixed by #2381
Labels
bug Potential issues with the zarr-python library

Comments

@DimitriPapadopoulos
Copy link
Contributor

DimitriPapadopoulos commented Oct 15, 2024

Zarr version

v3

Installation

sources

Description

These methods of abstract base class Store are non-async:

Child classes LocalStore, LoggingStore, RemoteStore, MemoryStore, ZipStore override these functions as async. Take the example of ZipStore:

Is that on purpose? I think async is part of the method signature and should be added to the base class.

@DimitriPapadopoulos DimitriPapadopoulos added the bug Potential issues with the zarr-python library label Oct 15, 2024
@jhamman
Copy link
Member

jhamman commented Oct 15, 2024

This is intentional but I don't remember all the details. It has to do with how Python does abstract methods for async generators IIRC. If there is a better solution, I'd be happy to see one come forward though.

@DimitriPapadopoulos
Copy link
Contributor Author

I'm afraid I don't have much experience with async methods, so I cannot help much.

It's just that:

I could start a PR where I try to add async to the abstract base class methods, so we can see what happens. How does it sound?

@jhamman
Copy link
Member

jhamman commented Oct 15, 2024

Go for it @DimitriPapadopoulos!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Potential issues with the zarr-python library
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants