update retrieve_data in era5.py #414
Conversation
- changed download_format to .zip - added entcrypting of .zip file from download and merging of contained .nc files - add url
for more information, see https://pre-commit.ci
|
This pull request should solve the issue mentioned in #413 |
| # Set url for data download, this allows to switch to different data | ||
| # sources more easily. | ||
| url = "https://cds.climate.copernicus.eu/api" |
There was a problem hiding this comment.
How would you switch to other sources? Could you explain?
That should probably be a constant at module level, and maybe an optional argument
There was a problem hiding this comment.
Hi Lukas, you are right, I just added it here for later improvement. For example, I have a local version in which I added the glofas data, but they are stored under a different url, it is not cds, but rather ads. Switching the url in the dataset is then just simpler then going to the "config" file of the cds-api. Furthermore, with a fixed url in the data retrieval, changes can be made more easily. It was just a comment here and will be taken up in future additions.
There was a problem hiding this comment.
Ok, that makes sense. But I don't think we want to hardcode the url in here without the ability to pass it as an argument. We are basically overriding any urls a user may have set up manually via the cds api config file without being able to change it. So just move it to an extra argument to get this merged. Future additions we can make anyways
This needs to be done, as the new cds api takes only string values for a valid data call.
for more information, see https://pre-commit.ci
|
Next to the comments, please add a release note. Afterwards we could merge this. Thank you ! |
Co-authored-by: Lukas Trippe <[email protected]>
There was a problem hiding this comment.
Thanks @TimFuermann!
As outlined in the corresponding issue, I would first like to reproduce the problem to see if this change is necessary.
Second thing is that I would like to see a brief performance check in terms of memory and time.
| "month": str(time[0].month), | ||
| "day": str(time[0].day), | ||
| "year": [str(time[0].year)], | ||
| "month": [str(time[0].month).zfill(2)], |
There was a problem hiding this comment.
If leading zeros are required, perhaps this is better (and "%d", "%Y" for day and year):
| "month": [str(time[0].month).zfill(2)], | |
| "month": [time[0].strftime("%m")], |
Closes # (if applicable).
Changes proposed in this Pull Request
Checklist
doc.environment.yaml,environment_docs.yamlandsetup.py(if applicable).doc/release_notes.rstof the upcoming release is included.