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

Some updates for new CDS beta backend API #85

Merged
merged 7 commits into from
Sep 11, 2024
Merged

Conversation

pont-us
Copy link
Member

@pont-us pont-us commented Jul 26, 2024

Closes #84 .

These changes don't yet provide full support for the new API version (in part because the API server is in beta and it's not yet clear whether some of its new behaviours are planned changes or temporary bugs), but with them applied all but one of the unit tests pass. test_open_data_null_variables_list fails because the API server returns a Zip rather than the requested NetCDF.

  • Require cdsapi >=0.7.0

  • Add an api_version parameter to ERA5DatasetHandler.init (defaulting to 1).

  • Support new data_format parameter (replaces old format) in ERA5DatasetHandler

  • In CDSDataOpener, support both old and new ERA5 time coordinate names (time and valid_time respectively).

pont-us and others added 2 commits July 26, 2024 15:58
These changes don't yet provide full support for the new API
version (in part because the API server is in beta and it's
not yet clear whether some of its new behaviours are planned
changes or temporary bugs), but with them applied all but
one of the unit tests pass. test_open_data_null_variables_list
fails because the API server returns a Zip rather than the
requested NetCDF.

- Require cdsapi >=0.7.0

- Add an api_version parameter to ERA5DatasetHandler.__init__
  (defaulting to 1).

- Support new `data_format` parameter (replaces old `format`) in
  ERA5DatasetHandler

- In CDSDataOpener, support both old and new ERA5 time coordinate
  names (`time` and `valid_time` respectively).
@konstntokas konstntokas marked this pull request as ready for review September 3, 2024 14:17
@konstntokas
Copy link
Contributor

The open parameter spatial_res is changed from a required to an optional parameter for ERA5. The default grid size is then returned. This allows to request larger datasets.

- Bump minimum cdsapi version to 0.7.2 in environment.yml

- Fix a typo in the readme

- Remove a superfluous blank cell from an example notebook
@konstntokas
Copy link
Contributor

konstntokas commented Sep 10, 2024

I tried to invoke an error, when the dimension do not fit. However, the examples which produced errors last week, work fine now. I even added an example for ERA5, where I used parameters from atmosphere and ocean. The grid resolution of the ocean parameters is coarser by a factor of two. xarray.open_mfdataset() can deal with this, since the coarser grid of the ocean parameter is a subset of the finer grid of the atmospheric parameter. The missing values are filled with nans.

So far, I could not find an example, which gives me an error. I remember that xarray.open_mfdataset() gave an understandable error, when the dimensions do not fit. I would say, it is enough to rely on this, or maybe add a better error message, when an actual example comes up. At the moment I have the feeling the CDS-beta is evolving quickly.

@pont-us
Copy link
Member Author

pont-us commented Sep 11, 2024

@konstntokas I agree that it's enough if things are working with the current backend state and our current tests and use cases. We'll deal with any new errors or problems as and when they come up. Test suite and notebooks have now finally run to completion and code looks fine, so I'll approve and merge.

@pont-us pont-us self-assigned this Sep 11, 2024
Copy link
Member Author

@pont-us pont-us left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, I can't officially approve since it's still technically my PR, so I'll just merge :)

@pont-us pont-us merged commit 580e3df into main Sep 11, 2024
2 checks passed
@pont-us pont-us deleted the pont-84-new-cds-api branch September 11, 2024 14:23
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.

Update xcube-cds to work with new CDS backend API
2 participants