burn_vector_geometry and polygonize #126
Conversation
| # | ||
| # In this example, we mark the faces that are covered by a certain province. | ||
|
|
||
| provinces = xu.data.provinces_nl().to_crs(28992) |
There was a problem hiding this comment.
Add comment why you call to_crs here, is it because the data is in a different CRS? Or because it has no CRS?
There was a problem hiding this comment.
It has to be reprojected from WGS84. I've added a paragraph of explanation.
examples/vector_conversion.py
Outdated
| # In this example, we mark the faces that are covered by a certain province. | ||
|
|
||
| provinces = xu.data.provinces_nl().to_crs(28992) | ||
| provinces["value"] = range(len(provinces)) |
There was a problem hiding this comment.
| provinces["value"] = range(len(provinces)) | |
| provinces["value"] = range(len(provinces)) # Give each province a unique label |
There was a problem hiding this comment.
Added to the extra paragraph.
| # | ||
| # The exterior boundaries of the province polygons will provide | ||
| # a collection of linestrings that we can burn into the grid: | ||
|
|
There was a problem hiding this comment.
For completeness, I think it is good to also add an example with points. For example, two points indicating the locations of the cities of Delft and Utrecht. Just to prove that xugrid can do it, and in what form points are expected (as GeoDataFrame containing shapely points).
There was a problem hiding this comment.
Done using the centroids of the provinces.
| faces: IntArray, | ||
| vertices: FloatArray, | ||
| ): | ||
| # TODO: move this into numba_celltree instead? |
There was a problem hiding this comment.
Probably good to add this TODO to all functions that need to be moved to numba_celltree, like point_in_triangle. This will help communicating to other developers which functions might need to be moved to numba_celltree. Also don't forget to open an issue for this!
|
|
||
| Parameters | ||
| ---------- | ||
| gdf: geopandas.GeoDataFrame |
There was a problem hiding this comment.
| gdf: geopandas.GeoDataFrame | |
| gdf: geopandas.GeoDataFrame | |
| Collection of polygons, points, and/or lines to be burned into the grid. |
| Parameters | ||
| ---------- | ||
| gdf: geopandas.GeoDataFrame | ||
| like: UgridDataArray, UgridDataset, or Ugrid2d |
There was a problem hiding this comment.
| like: UgridDataArray, UgridDataset, or Ugrid2d | |
| like: UgridDataArray, UgridDataset, or Ugrid2d | |
| Grid to burn vector data into. |
| def test_burn_polygons(grid, polygons_and_values): | ||
| polygons, values = polygons_and_values | ||
| output = np.full(grid.n_face, np.nan) | ||
| burn._burn_polygons(polygons, grid, values, all_touched=False, output=output) |
There was a problem hiding this comment.
For completeness, also add a test where all_touched=True
xugrid/ugrid/burn.py
Outdated
|
|
||
| def burn_vector_geometry( | ||
| gdf: "geopandas.GeoDataframe", # type: ignore # noqa | ||
| like: "xugrid.Ugrid2d", |
There was a problem hiding this comment.
I was a bit in doubt about the name like here, conceptually I perceived this method different than regrid_like or full_like. As it is returning a copy of like however, it is essentially doing such a thing, but it feels a bit unintuitive. I cannot think of a better name though. grid doesn't cover it (as UgridDataset is also allowed), and template introduces suddenly new nomenclature to the package. Just sharing my thoughts, you can resolve without notice.
xugrid/ugrid/burn.py
Outdated
|
|
||
| def burn_vector_geometry( | ||
| gdf: "geopandas.GeoDataframe", # type: ignore # noqa | ||
| like: "xugrid.Ugrid2d", |
There was a problem hiding this comment.
| like: "xugrid.Ugrid2d", | |
| like: Union["xugrid.Ugrid2d", "xugrid.UgridDataArray", "xugrid.UgridDataset"], |
Add vector utilities such as provided by GDAL for raster data.