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
Bug #109 forcing with DatetimeNoLeap #110
Conversation
@DirkEilander : could you review this small bugfix? You reviewed this new change from #97 so you should already know more or less what it's about :) Many thanks! |
Tests should be fixed when merging Deltares/hydromt#211 in hydromt |
The test fails on another issue it seems, see #113 |
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.
Thanks for these fixes @hboisgon.
I think it's basically good to go. But strongly suggest to remove the lines that assume self.crs still has the rasterio CRS dtype as these will never be hit (see comment)
hydromt_wflow/wflow.py
Outdated
elif hasattr(self.crs, "is_epsg_code"): | ||
if self.crs.is_epsg_code: | ||
code = int(self.crs["init"].lstrip("epsg:")) |
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.
I think these lines can be removed because self.crs is always of pyproj.CRS type.
For the RasterModel see https://github.com/Deltares/hydromt/blob/075ac08d91f7e06bdc0504c0163d450e7b23a955/hydromt/models/model_grid.py#L181
For the Model see
https://github.com/Deltares/hydromt/blob/075ac08d91f7e06bdc0504c0163d450e7b23a955/hydromt/models/model_api.py#L1278
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.
This is true for hydromt >= 0.5.0 but not for older version I think. I don't mind removing I think we put pins everywhere already in the different env files.
See #109 : solution is to use xr.to_datetimeindex instead of pd.to_datetime to convert times in forcing DataArrays