-
Notifications
You must be signed in to change notification settings - Fork 28
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
update to the official new CDS #44
Conversation
+ autoget.py: - update `cds_url` link - update to the latest syntax of `cdsapi` for the request dictionary input - use `f'{}'` instead of `.format` syntax + README.md: update account setup doc for the official new CDS + requirements.txt: the new CDS requires cdsapi>=0.7.2 + update `model.cfg` and `tests/test_dload.py` for the new CDS + processor.intP2H(): add whitespace for better readability
Reviewer's Guide by SourceryThis pull request updates the package to be compatible with the official new Copernicus Climate Data Store (CDS). This includes updating the CDS URL, updating the cdsapi syntax, updating the account setup documentation, and updating the required version of cdsapi. Sequence diagram for ERA5 data download with updated CDS APIsequenceDiagram
participant User
participant PyAPS3
participant CDS
User->>PyAPS3: Call ECMWFdload()
PyAPS3->>CDS: Connect to new CDS URL
Note over PyAPS3,CDS: https://cds.climate.copernicus.eu/api
PyAPS3->>CDS: Authenticate with Personal Access Token
PyAPS3->>CDS: Send updated request format
Note over PyAPS3,CDS: New dictionary structure with list values
CDS-->>PyAPS3: Return requested data
PyAPS3-->>User: Save data to file
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
PR SummaryThis Pull Request updates the codebase to align with the new Copernicus Climate Data Store (CDS) infrastructure. Key changes include:
These changes ensure that the project remains functional and up-to-date with the latest CDS infrastructure, improving both usability and maintainability. Suggestion
This comment was generated by AI. Information provided may be incorrect. Current plan usage: 0% Have feedback or need help? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @yunjunz - I've reviewed your changes - here's some feedback:
Overall Comments:
- In autoget.py, the check 'if model in "ERA5"' could lead to unintended matches if model values vary. Consider using an explicit equality check (e.g. 'if model == "ERA5"') to ensure clarity and correctness.
- Verify that the changes in the request dictionary (switching from string values to list values for parameters like year, month, day, and time) are fully compatible with the new CDS API. If not already documented, a brief inline comment for this change would help future maintainers.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @yunjunz , thanks for the PR to update the official urls. The links are working for me. The PR looks good.
autoget.py
:cds_url
linkcdsapi
for the request dictionary inputf'{}'
instead of.format
syntaxREADME.md
: update account setup doc for the official new CDSrequirements.txt
: the new CDS requires cdsapi>=0.7.2update
model.cfg
andtests/test_dload.py
for the new CDSprocessor.intP2H()
: add whitespace for better readabilitySummary by Sourcery
Update to the official new Copernicus Data Store (CDS).
Enhancements:
autoget.py
script to use the new CDS URL and the latestcdsapi
syntax, also replaced the.format
syntax with f-strings.processor.intP2H()
function by adding whitespace.Documentation:
Tests: