-
Notifications
You must be signed in to change notification settings - Fork 54
Change default loadable_variables (and indexes) to match xarray's behaviour #477
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
Merged
TomNicholas
merged 63 commits into
zarr-developers:develop
from
TomNicholas:refactor_loadable_variables
Mar 24, 2025
Merged
Changes from all commits
Commits
Show all changes
63 commits
Select commit
Hold shift + click to select a range
99a99aa
draft refactor
TomNicholas feadc32
sketch of simplified handling of loadable_variables
TomNicholas b6e0242
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] f284c5a
get at least some tests working
TomNicholas 7d50f8e
separate VirtualBackend api definition from common utilities
TomNicholas 618da43
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] 7f0ee4d
remove indexes={} everywhere in tests
TomNicholas 6e020d3
Merge branch 'refactor_loadable_variables' of https://github.com/TomN…
TomNicholas f85c9d9
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] 03e50fc
stop passing through loadable_variables to where it isn't used
TomNicholas cc34f77
implement logic to load 1D dimension coords by default
TomNicholas eefbcc8
remove more instances of indexes={}
TomNicholas e05a640
remove more indexes={}
TomNicholas 1106a40
refactor logic for choosing loadable_variables
TomNicholas dd0c947
fix more tets
TomNicholas b3f445a
xfail Aimee's test that I don't understand
TomNicholas be3429f
xfail test that explicitly specifies no indexes
TomNicholas ce4ea4e
made a bunch more stuff pass
TomNicholas bb23c7b
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] 1ea236f
fix netcdf3 reader
TomNicholas 91b4dd2
Merge branch 'refactor_loadable_variables' of https://github.com/TomN…
TomNicholas 30d020e
fix bad import in FITS reader
TomNicholas 8a76cfe
fix import in tiff reader
TomNicholas 8b0e7ae
fix import in icechunk test
TomNicholas ae5f480
Merge branch 'main' into refactor_loadable_variables
TomNicholas 9b01010
release note
TomNicholas ef0ab78
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] 80faea6
update docstring
TomNicholas c85538a
Merge branch 'refactor_loadable_variables' of https://github.com/TomN…
TomNicholas 8a5fc65
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] 0162bac
fix fits reader
TomNicholas 2cea749
xfail on empty dict for indexes
TomNicholas 216edbd
linting
TomNicholas fbcd127
Merge branch 'refactor_loadable_variables' of https://github.com/TomN…
TomNicholas e14f173
actually test new expected behaviour
TomNicholas 97e1f74
fix logic for setting loadable_variables
TomNicholas 77a7227
update docs page to reflect new behaviour
TomNicholas c2ecd88
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] d31a9fd
fix expected behaviour in another tests
TomNicholas 6fa03f8
additional assert
TomNicholas eb773b7
Merge branch 'develop' into refactor_loadable_variables
maxrjones e2c3a79
Merge branch 'develop' into refactor_loadable_variables
maxrjones 2859052
Merge branch 'refactor_loadable_variables' of https://github.com/TomN…
TomNicholas 6bfcf1c
Merge branch 'develop' into refactor_loadable_variables
TomNicholas 5820895
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] 6c049f0
use encode_dataset_coordinates in kerchunk writer
TomNicholas ef4fa81
Merge branch 'develop' into refactor_loadable_variables
TomNicholas 7729a33
Encode zarr vars
maxrjones 51fa125
Merge pull request #1 from maxrjones/fixup
TomNicholas a00c097
fix some mypy errors
TomNicholas 0d6fb40
move drop_variables implmentation to the end of every reader
TomNicholas d15ed90
override loadable_variables and raise warning
TomNicholas 4ccbb5b
fix failing test by not creating loadable variables that would get in…
TomNicholas 7992c08
improve error message
TomNicholas f20af13
remove some more occurrences of indexes={}
TomNicholas 33d45c2
skip slow test
TomNicholas 917a973
slay mypy errors
TomNicholas 94f3d4f
docs typos
TomNicholas 72beb94
should fix dmrpp test
TomNicholas 83cb2a5
Merge branch 'refactor_loadable_variables' of https://github.com/TomN…
TomNicholas 8a436f2
Delete commented-out code
TomNicholas 9470b97
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] 6bf2615
remove unecessary test skip
TomNicholas File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| from abc import ABC | ||
| from collections.abc import Iterable, Mapping | ||
| from typing import Optional | ||
|
|
||
| import xarray as xr | ||
|
|
||
|
|
||
| class VirtualBackend(ABC): | ||
| @staticmethod | ||
| def open_virtual_dataset( | ||
| filepath: str, | ||
| group: str | None = None, | ||
| drop_variables: Iterable[str] | None = None, | ||
| loadable_variables: Iterable[str] | None = None, | ||
| decode_times: bool | None = None, | ||
| indexes: Mapping[str, xr.Index] | None = None, | ||
| virtual_backend_kwargs: Optional[dict] = None, | ||
| reader_options: Optional[dict] = None, | ||
| ) -> xr.Dataset: | ||
| raise NotImplementedError() | ||
|
|
||
| @staticmethod | ||
| def open_virtual_datatree( | ||
| path: str, | ||
| group: str | None = None, | ||
| drop_variables: Iterable[str] | None = None, | ||
| loadable_variables: Iterable[str] | None = None, | ||
| decode_times: bool | None = None, | ||
| indexes: Mapping[str, xr.Index] | None = None, | ||
| virtual_backend_kwargs: Optional[dict] = None, | ||
| reader_options: Optional[dict] = None, | ||
| ) -> xr.DataTree: | ||
| raise NotImplementedError() |
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Required otherwise we get inlined variables in the kerchunk file which we don't know how to read (#489)