Deltares / xugrid

Xarray and unstructured grids
https://deltares.github.io/xugrid/
MIT License
58 stars 8 forks source link

Pass `mk.MeshKernel` instead of `mk.mesh2d` to `xu.Ugrid2d.from_meshkernel` #188

Open veenstrajelmer opened 8 months ago

veenstrajelmer commented 8 months ago

This way it is possible to derive the projected bool automatically, since it is set in the mk.MeshKernel instance (albeit indirectly via get_projection(). It would avoid the need to derive the projected bool from the mk.MeshKernel instance before converting the mesh2d to Ugrid2d and limits the amount of input arguments.

E.g. in dfm_tools we derive projected from the mk.MeshKernel instance before passing it to from_meshkernel: https://github.com/Deltares/dfm_tools/blob/9a527af6f0df6529aecf46d434315bb277158797/dfm_tools/xugrid_helpers.py#L550-L557 In https://github.com/Deltares/dfm_tools/pull/685 we moved to deriving crs and projected from uds instead of from mk.MeshKernel

And in hydromt_delft3dfm we pass projected and crs, altough projected is derived from crs before passing it to from_meshkernel: https://github.com/Deltares/hydromt_delft3dfm/blob/a97d2b491501e1fc78042e58292405e8bbcd2baa/hydromt_delft3dfm/mesh_utils.py#L259-L267

After implementation, it is still important to check if the passed crs and mk.MeshKernel are both geographic or both projected. But that is slighly better than the current situation, since we should actually check whether crs and mk.MeshKernel and projected are aligned.

The above also goes for mesh1d/Ugrid1d.

Alternatively Alternatively, derive projected from crs. This would avoid the need to pass a different meshkernel instance. In that case it might be easiest to pick it up in https://github.com/Deltares/xugrid/issues/187

More info Another consideration is consistency. We have back-forth conversions for meshkernel with meshkernel and from_meshkernel. These functions are available for UgridBase, Ugrid1d and Ugrid2d. meshkernel always creates a MeshKernel() instance, but from_meshkernel expects a mesh2d or mesh1d. So for consistency it would make sense to go for the first approach (and check the geographic property of the meshkernel instance against that of the passed crs).