ecmwf / cfgrib

A Python interface to map GRIB files to the NetCDF Common Data Model following the CF Convention using ecCodes
Apache License 2.0
409 stars 77 forks source link

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

Closed Metamess closed 1 year ago

Metamess commented 1 year ago

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 commented 1 year ago

Codecov Report

Patch coverage: 25.00% and project coverage change: -41.73% :warning:

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

Additional details and impacted files ```diff @@ 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](https://app.codecov.io/gh/ecmwf/cfgrib/pull/349?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ecmwf) | Coverage Δ | | |---|---|---| | [cfgrib/\_\_main\_\_.py](https://app.codecov.io/gh/ecmwf/cfgrib/pull/349?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ecmwf#diff-Y2ZncmliL19fbWFpbl9fLnB5) | `38.88% <0.00%> (-55.56%)` | :arrow_down: | | [cfgrib/xarray\_store.py](https://app.codecov.io/gh/ecmwf/cfgrib/pull/349?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ecmwf#diff-Y2ZncmliL3hhcnJheV9zdG9yZS5weQ==) | `4.61% <0.00%> (-93.85%)` | :arrow_down: | | [tests/test\_50\_sample\_data.py](https://app.codecov.io/gh/ecmwf/cfgrib/pull/349?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ecmwf#diff-dGVzdHMvdGVzdF81MF9zYW1wbGVfZGF0YS5weQ==) | `13.95% <0.00%> (-86.05%)` | :arrow_down: | | [tests/test\_40\_xarray\_store.py](https://app.codecov.io/gh/ecmwf/cfgrib/pull/349?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ecmwf#diff-dGVzdHMvdGVzdF80MF94YXJyYXlfc3RvcmUucHk=) | `5.05% <20.00%> (-94.95%)` | :arrow_down: | | [cfgrib/dataset.py](https://app.codecov.io/gh/ecmwf/cfgrib/pull/349?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ecmwf#diff-Y2ZncmliL2RhdGFzZXQucHk=) | `91.04% <100.00%> (-7.41%)` | :arrow_down: | ... and [15 files with indirect coverage changes](https://app.codecov.io/gh/ecmwf/cfgrib/pull/349/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ecmwf)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

iainrussell commented 1 year ago

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

iainrussell commented 1 year ago

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.

Metamess commented 1 year ago

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