Skip to content
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

Add is_geographic or is_projected boolean property to meshkernel instance #108

Closed
veenstrajelmer opened this issue Nov 13, 2023 · 1 comment
Labels
wontfix This will not be worked on

Comments

@veenstrajelmer
Copy link
Collaborator

veenstrajelmer commented Nov 13, 2023

Is your feature request related to a problem? Please describe.
Quick question. When we want to save a meshkernel instance as a netcdf file, it is good to add "projection_x_coordinate" or "longitude" and other attributes. For this we need to know whether the meshkernel instance is projected/cartesian or spherical. Whether it is spherical or spherical accurate is irrelevant in this case, since the attributes would be the same. Maybe it is also useful to add a crs (coordinate reference system) to the meshkernel instance, but that might be out of the scope of meshkernel.

Describe the solution you'd like
I would like to avoid checking on mk.get_projection()==meshkernel.ProjectionType.CARTESIAN in several packages (dfm_tools, xugrid, hydromt, hydrolib-core and maybe in the future external packages also). Is there a boolean property of the meshkernel instance that provides this information? If not, could it be created?

For instance, when converting a meshkernel instance to xugrid.UgridDataset (xugrid.Ugrid2d) with xugrid.Ugrid2d.from_meshkernel() we need to know it:
https://github.com/Deltares/xugrid/blob/a021f190bc35dae4b325a11569f9d87d600bc9be/xugrid/ugrid/ugrid2d.py#L193

Alternatives
Alternatively, we can limit the packages where this check is carried out. One way would be to not pass projected to
xugrid.Ugrid2d.from_meshkernel, but not sure yet if that is desireable: Deltares/xugrid#188

@veenstrajelmer veenstrajelmer added the enhancement New feature or request label Nov 13, 2023
@veenstrajelmer veenstrajelmer changed the title Add is_geographic or is_projected property to meshkernel instance Add is_geographic or is_projected boolean property to meshkernel instance Nov 13, 2023
@ahmad-el-sayed
Copy link
Contributor

The suggested solution does not make sense because more than 2 projection types are possible in the backend. Currently 2 are fully supported in MKPy for the time being. What you can do is write a small function that does that:

def projection_is_cartesian(projection_type: ProjectionType):
  return projection_type == meshkernel.ProjectionType.CARTESIAN

Then use it as follows: if(projection_is_cartesian(mk.get_projection())): ...

@ahmad-el-sayed ahmad-el-sayed closed this as not planned Won't fix, can't repro, duplicate, stale Nov 28, 2023
@ahmad-el-sayed ahmad-el-sayed added wontfix This will not be worked on and removed enhancement New feature or request labels Nov 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

2 participants