Open
Conversation
github-actions
bot
added
topic-backends
topic-zarr
wavebyrd
force-pushed
the
fix-open-zarr-typing
branch
from
7889687 to
8d80b04
Compare
github-actions
bot
added
topic-indexing
topic-CF conventions
topic-groupby
topic-dask
topic-plotting
topic-testing
topic-performance
topic-cftime
topic-arrays
Author
|
Friendly ping on this type annotation PR for open_zarr. Let me know if any changes needed! |
Collaborator
|
Can you rebase that on main? I'm not looking through 200k LOC changes, haha. |
All parameters in open_zarr were untyped, which caused type checkers like pyright to reject valid arguments (e.g. passing "auto" for chunks). Adds annotations to every parameter and the return type, using T_Chunks for the chunks parameter and matching types from the docstrings for the rest. Fixes pydata#11221
wavebyrd
force-pushed
the
fix-open-zarr-typing
branch
from
8d80b04 to
2b191dd
Compare
Author
|
Rebased onto main - now shows only the single commit for this fix. Thanks for the review! |
headtr1ck
approved these changes
Mar 19, 2026
| mask_and_scale=True, | ||
| decode_times=True, | ||
| concat_characters=True, | ||
| store: str | os.PathLike[Any] | MutableMapping | None, |
Collaborator
There was a problem hiding this comment.
Could you add the generic types of MutableMapping?
I don't actually know what it should be, probably MutableMapping[str, str]?
Collaborator
There was a problem hiding this comment.
Actually mypy complains here. Maybe you can simply use xarray.backends.common.T_PathFileOrDataStore?
Collaborator
|
Feel free to add this change to what's new! |
headtr1ck
reviewed
Mar 21, 2026
| group: str | None = None, | ||
| synchronizer: object | None = None, | ||
| chunks: T_Chunks | Default = _default, | ||
| decode_cf: bool = True, |
Collaborator
There was a problem hiding this comment.
Mypy says that this can also take a CFTimeCoder?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds type annotations to all parameters of
open_zarrand its return type.Previously the function was essentially untyped, which caused type checkers like pyright to reject valid arguments such as
chunks="auto"because the default value_default(of typeDefault) was the only inferred type.chunksis now typed asT_Chunks | DefaultDatasetFixes #11221