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

Renamed 'grib_errors' parameter to 'errors' to match open_file() signature #349

Merged
merged 9 commits into from Aug 18, 2023

Conversation

Metamess
Copy link
Contributor

Closes #348

Fixes a bug where providing "grib_errors" as key in backend_kwargs is required by some function (open_variable_datasets()) but causes a TypeError due to not being a recognized parameter in others (CfGribBackend.open_dataset()).

Also fixes a test (test_40_xarray_store.py::test_open_dataset_corrupted()) to specifically expect the type of Error associated with the expected issue, which was masking the TypeError.

Additionally, also adds the following line to test_40_xarray_store.py::test_open_dataset():
xarray_store.open_dataset(TEST_DATA, backend_kwargs={"errors": "raise"})
This tests that calling open_dataset() with a known-good GRIB file and "errors": "raise" does not, in fact, raise an Error.

@codecov-commenter
Copy link

codecov-commenter commented Aug 17, 2023

Codecov Report

Patch coverage: 25.00% and project coverage change: -41.73% ⚠️

Comparison is base (177ac62) 95.65% compared to head (d4e003c) 53.93%.

Additional details and impacted files
@@             Coverage Diff             @@
##           master     #349       +/-   ##
===========================================
- Coverage   95.65%   53.93%   -41.73%     
===========================================
  Files          26       26               
  Lines        2073     2073               
  Branches      238      231        -7     
===========================================
- Hits         1983     1118      -865     
- Misses         59      937      +878     
+ Partials       31       18       -13     
Files Changed Coverage Δ
cfgrib/__main__.py 38.88% <0.00%> (-55.56%) ⬇️
cfgrib/xarray_store.py 4.61% <0.00%> (-93.85%) ⬇️
tests/test_50_sample_data.py 13.95% <0.00%> (-86.05%) ⬇️
tests/test_40_xarray_store.py 5.05% <20.00%> (-94.95%) ⬇️
cfgrib/dataset.py 91.04% <100.00%> (-7.41%) ⬇️

... and 15 files with indirect coverage changes

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

@iainrussell
Copy link
Member

This is a really nice PR, @Metamess, I'm just trying to sort out the CI before merging.

tests/test_40_xarray_store.py Show resolved Hide resolved
@iainrussell
Copy link
Member

Thank you very much for this PR - you also mentioned that the test environment needed to be updated and you were right, it did just in order for this to work! That was because it required a newer ecCodes, which in turn had some new requirements, and was not supported by Python 3.7.

@iainrussell iainrussell merged commit ce74335 into ecmwf:master Aug 18, 2023
8 checks passed
@Metamess
Copy link
Contributor Author

Thanks a lot for all the additional work Iain! Glad I could be of help 😁

@Metamess Metamess deleted the 348-grib-errors-parameter branch August 21, 2023 07:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inconsistency between "grib_errors" and "errors" as argument name
3 participants