Pyogrio io #583
Pyogrio io #583
Conversation
There was a problem hiding this comment.
Thanks for this PR!
Generally it looks great and thanks again for checking all the different options I suggested :)
I have two final suggestions. Furthermore, I think it would be good if @savente93 could have a final look at the code syntax (I didn't pay much attention to this because of time).
hydromt/gis_utils.py
Outdated
| @@ -553,3 +558,46 @@ def to_geographic_bbox(bbox, source_crs): | |||
| bbox = Transformer.from_crs(source_crs, target_crs).transform_bounds(*bbox) | |||
|
|
|||
| return bbox | |||
|
|
|||
|
|
|||
| def prepare_pyogrio_reader_filters( | |||
There was a problem hiding this comment.
I think this method could be more widely used than for pyogrio input. Perhaps renaming it to parse_bbox or something more generic would make that more clear.
pyproject.toml
Outdated
| @@ -25,6 +25,7 @@ dependencies = [ | |||
| "rioxarray", # wraps rasterio and xarray. Almost superceded by hydromt/raster.py | |||
| "pandas", # Dataframes | |||
| "pyflwdir>=0.5.4", # Hight models and derivatives | |||
| "pyogrio>=0.6", # io for geopandas dataframes | |||
There was a problem hiding this comment.
Similar to other optional io packages I would suggest to move pyogrio to the optional io dependencies until it the default engine of geopandas.
|
I'll be reviewing it in a few minutes, but in the mean time I have one questuion:
I missed this, is it because of this issue, or because of something else? |
Issue addressed
Fixes #575
Explanation
pyogriowill be replacing geopandas read models in v1.0. This library does not try to doPath.expanduser, so using this version fixes the bug for me. Masking still does not work in that version, so I had to do some work myself (filtering input / output).Checklist
main