-
Notifications
You must be signed in to change notification settings - Fork 22
Update output.csv.map if pointing to lake.locs #530
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
Conversation
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.
Pull Request Overview
Updates the v0 to v1 upgrade logic to handle CSV column map references that point to lake or reservoir locations, converting them to the new "reservoir_location__count" format.
- Adds handling for "lateral.river.lake.locs" and "lateral.river.reservoir.locs" map references in CSV output configuration
- Updates test data files to reflect the new mapping format
- Refactors code style by removing unnecessary
.keys()
calls and using walrus operator
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
hydromt_wflow/version_upgrade.py | Adds logic to convert lake/reservoir location maps to "reservoir_location__count" |
tests/data/wflow_v0x/sbm/wflow_sbm_v1.toml | Updates test data with expected v1 format output |
examples/data/wflow_upgrade/sbm/wflow_sbm_v0x.toml | Updates example data with v0 format input for testing |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
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.
All good! I forgot but could you do the same for output.netcdf_scalar? (netcdf in v0)
Fixes #439 v0 to v1 upgrade now also checks for csv.column.map and updates it in the new output variable output.csv.column.map. "lateral.river.lake.locs" and "lateral.river.reservoir.locs" become "reservoir_location__count"
0ff633a
to
c6065ec
Compare
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.
Sorry one more thing I didn't see before... but for the rest it would be good to merge!
|
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.
LGTM!
Fixes #493
v0 to v1 upgrade now also checks for csv.column.map and updates it in the new output variable output.csv.column.map.
"lateral.river.lake.locs" and "lateral.river.reservoir.locs" become "reservoir_location__count"
Issue addressed
Fixes #
Explanation
Explain how you addressed the bug/feature request, what choices you made and why.
Checklist
main
Additional Notes (optional)
Add any additional notes or information that may be helpful.