-
Notifications
You must be signed in to change notification settings - Fork 6
Issue #949 allocate river layers #952
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
…tage, dim order and/or grid type was not preserved.
luitjansl
left a comment
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.
looks good, some comments
| top of model layers | ||
| bottom: GridDataArray | ||
| bottom of model layers | ||
| bottom_elevation: GridDataArray |
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.
river_bottom_elevation?
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.
bottom_elevation is used in the River package, wanted to stick to that convention as much as possible.
| top: GridDataArray, | ||
| bottom: GridDataArray, | ||
| head: GridDataArray, | ||
| ) -> GridDataArray: |
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.
for all these allocation functions, input arrays like "top" and "bottom" should all be xr.DataArray or xu.UgridDataArray but they should (I assume) be all of the same type. If so, could you do some validation? you can probably use the same validation function for all allocators.
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.
Not necessarily true:
You can supply top and bottom as scalar xr.DataArray and 1D xr.DataArray with layer dimension and provide unstructured grids to the other variables.
Fixes #949
Description
polygonizeutil function to work around a nasty circular import. The solution also prevents changes to the public API.Checklist
Issue #nr, e.g.Issue #737