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

Move to new CDS, prep for 2.0.0b1 release #166

Merged
merged 15 commits into from
Aug 30, 2024
Merged

Move to new CDS, prep for 2.0.0b1 release #166

merged 15 commits into from
Aug 30, 2024

Conversation

BSchilperoort
Copy link
Member

@BSchilperoort BSchilperoort commented Jul 31, 2024

See #165

Should work with new CDS. Updated the version to 2.0.0b1 & added warnings for users.

When the old CDS is shut down on 3 September, I'll yank the old version on PyPI (see #167).

Also, as this will be v2.0.0, I removed long deprecated parts of the code (prelimbe, orography), and made "--splitmonths True" the default

@BSchilperoort
Copy link
Member Author

Will pick this up again late August. Hopefully the new CDS will work by then.

@BSchilperoort BSchilperoort marked this pull request as ready for review August 28, 2024 09:33
@BSchilperoort
Copy link
Member Author

@malininae can you try out this PR and review it? I'll make a beta release of era5cli once this is merged.

@malininae
Copy link
Contributor

@BSchilperoort Thanks so much! First question, it seems that by running era5cli config --key "herewasmykey", there is a request sent for the 2t at 14:00 on the 1st of December 2012. Originally I thought something went wrong, maybe my old .cdsapirc was screwing something up or something, so removed both .config/era5cli/cds_key.txt and .cdsapirc and ended both times sending this request
image
I briefly looked through config.py but couldn't find the request. Is that an expected behavior?

@malininae
Copy link
Contributor

@BSchilperoort by sending the era5cli hourly --variables maximum_2m_temperature_since_previous_post_processing --startyear 2022 --endyear 2022 --format netcdf request, in my requests on https://cds-beta.climate.copernicus.eu/requests?tab=all I end up with the additional request for the 2t 2012-12-01 at 14:00. If using --splitmonths True, only one request is sent, not 12, which is good, but still annoying since the older version wasn't doing that. It seems that the issue is coming from era5cli side because I tried using suggested by the website

import cdsapi

dataset = "reanalysis-era5-single-levels"
request = {
    'product_type': ['reanalysis'],
    'variable': ['minimum_2m_temperature_since_previous_post_processing'],
    'year': ['2017'],
    'month': ['01', '02', '03', '04', '05', '06', '07', '08', '09', '10', '11', '12'],
    'day': ['01', '02', '03', '04', '05', '06', '07', '08', '09', '10', '11', '12', '13', '14', '15', '16', '17', '18', '19', '20', '21', '22', '23', '24', '25', '26', '27', '28', '29', '30', '31'],
    'time': ['00:00', '01:00', '02:00', '03:00', '04:00', '05:00', '06:00', '07:00', '08:00', '09:00', '10:00', '11:00', '12:00', '13:00', '14:00', '15:00', '16:00', '17:00', '18:00', '19:00', '20:00', '21:00', '22:00', '23:00'],
    'data_format': 'netcdf',
    'download_format': 'unarchived'
}

client = cdsapi.Client()
client.retrieve(dataset, request).download()

and it sent only the appropriate request.

Also, minor, request seem not be deleted on the cds once they are done. Not sure where that is coming from era5cli or cdsapi . The version 1.4 used to delete a request on the website. Again, thanks a lot for your work on this!

@BSchilperoort
Copy link
Member Author

BSchilperoort commented Aug 29, 2024

Thanks for having a look!

there is a request sent for the 2t at 14:00 on the 1st of December 2012.

That's intended behavior. It's a small (1.75MB & should be quick) request to ensure that the entered key is valid. This has been part of the code since v1.4 (early 2023).

Also, minor, request seem not be deleted on the cds once they are done. Not sure where that is coming from era5cli or cdsapi . The version 1.4 used to delete a request on the website.

This is most likely due to a change in the behavior of the cdsapi. The new cdsapi is a legacy wrapper around the new client code "cads-api-client". The cads-api-client has more features, but moving to that client would take more time than I can spend on getting era5cli working again.

Requests cannot be deleted with the legacy api: https://github.com/ecmwf-projects/cads-api-client/blob/main/cads_api_client/legacy_api_client.py#L68

If using --splitmonths True, only one request is sent, not 12, which is good, but still annoying since the older version wasn't doing that.

Hm. That's not the intended behavior. It should send separate requests. The splitmonths argument is designed to avoid exceeding the maximum request size, for example when downloading era5-land or ensembles. I am not sure if the maximum request size has changed with the new beta cds.

Running era5cli hourly --land --variables 2m_temperature --startyear 2000 --endyear 2000 --splitmonths True --area 53.6 3.3 50.7 7.5 --dryrun gives me 12 separate requests

@BSchilperoort
Copy link
Member Author

It's a small (should be quick) request to ensure that the entered key is valid

With the new cads-api-client we can do:

import cads_api_client
client = cads_api_client.ApiClient(key, url)
client.check_authentication()

This functionality was added in ecmwf-projects/cads-api-client#63 but still has to be included in a new release.

Copy link

codecov bot commented Aug 29, 2024

Codecov Report

Attention: Patch coverage is 86.48649% with 5 lines in your changes missing coverage. Please review.

Project coverage is 98.95%. Comparing base (f211c70) to head (e96ee1a).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #166      +/-   ##
==========================================
- Coverage   99.66%   98.95%   -0.72%     
==========================================
  Files          14       13       -1     
  Lines         601      574      -27     
==========================================
- Hits          599      568      -31     
- Misses          2        6       +4     

@malininae
Copy link
Contributor

Running era5cli hourly --land --variables 2m_temperature --startyear 2000 --endyear 2000 --splitmonths True --area 53.6 3.3 50.7 7.5 --dryrun gives me 12 separate requests

Sorry, I wasn't clear, it doesn't give 12 test requests, the 'real' requests are good, there are 12 of them.

Copy link
Contributor

@malininae malininae left a comment

Choose a reason for hiding this comment

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

@BSchilperoort I tried a few things. From my side, all looks good. Maybe someone else should also look as well.

@BSchilperoort
Copy link
Member Author

BSchilperoort commented Aug 30, 2024

@BSchilperoort I tried a few things. From my side, all looks good. Maybe someone else should also look as well.

Thanks for the review! I will release a beta version of the package. If issues arise I hope users will report them.

@BSchilperoort BSchilperoort linked an issue Aug 30, 2024 that may be closed by this pull request
@BSchilperoort BSchilperoort changed the title Move to new CDS Move to new CDS, prep for 2.0.0b1 release Aug 30, 2024
@BSchilperoort BSchilperoort merged commit 52a6a2d into main Aug 30, 2024
6 of 8 checks passed
@BSchilperoort BSchilperoort deleted the new-cds branch August 30, 2024 12:02
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.

Deprecate preliminary back extension support
2 participants