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

Add virtual_backend_kwargs argument to open_virtual_dataset #315

Merged
merged 5 commits into from
Nov 27, 2024

Conversation

TomNicholas
Copy link
Member

For #243 (comment) we need to pass a fs_root kwarg down to some but not all readers. We will have other reader-specific kwargs like this come up in future too. To be in keeping with xarray.open_dataset's API these should be passed as a dict called reader_kwargs or backend_kwargs.

However there is an existing reader_options kwarg, which is meant to be specifically for fsspec options. Having both reader_options and reader_kwargs would be confusing, so should we
a) rename reader_options to reader_kwargs, having it include fsspec options too,
b) have reader_kwargs and fsspec_kwargs, deprecating reader_options?

So far this PR just adds a reader_kwargs option to all backends, but raises if it is used.

cc @norlandrhagen @sharkinsspatial

  • Closes #xxxx
  • Tests added
  • Tests passing
  • Full type hint coverage
  • Changes are documented in docs/releases.rst
  • New functions/methods are listed in api.rst
  • New functionality has documentation

Copy link

codecov bot commented Nov 23, 2024

Codecov Report

Attention: Patch coverage is 37.50000% with 10 lines in your changes missing coverage. Please review.

Project coverage is 93.30%. Comparing base (3348670) to head (41ec08f).

Files with missing lines Patch % Lines
virtualizarr/readers/dmrpp.py 0.00% 2 Missing ⚠️
virtualizarr/readers/tiff.py 0.00% 2 Missing ⚠️
virtualizarr/readers/fits.py 50.00% 1 Missing ⚠️
virtualizarr/readers/hdf/hdf.py 50.00% 1 Missing ⚠️
virtualizarr/readers/hdf5.py 50.00% 1 Missing ⚠️
virtualizarr/readers/kerchunk.py 50.00% 1 Missing ⚠️
virtualizarr/readers/netcdf3.py 50.00% 1 Missing ⚠️
virtualizarr/readers/zarr_v3.py 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #315      +/-   ##
==========================================
+ Coverage   84.62%   93.30%   +8.68%     
==========================================
  Files          48       48              
  Lines        3362     3378      +16     
==========================================
+ Hits         2845     3152     +307     
+ Misses        517      226     -291     
Flag Coverage Δ
unittests 93.30% <37.50%> (+8.68%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

@TomNicholas
Copy link
Member Author

I've changed this PR to use virtual_backend_kwargs instead of reader_kwargs, which is closer to xarray's equivalent backend_kwargs, and also makes it less similar to reader_options. We should discuss this choice before implementing #245 though (as then it will become harder to change).

@TomNicholas TomNicholas merged commit 5152624 into main Nov 27, 2024
11 checks passed
@TomNicholas TomNicholas deleted the reader_kwargs branch November 27, 2024 18:00
@TomNicholas TomNicholas changed the title Add reader_kwargs argument to open_virtual_dataset Add virtual_backend_kwargs argument to open_virtual_dataset Nov 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant