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

Use a dummy CRS instead of the Dutch CRS in ribasim testmodels #1362

Closed
evetion opened this issue Apr 8, 2024 · 4 comments
Closed

Use a dummy CRS instead of the Dutch CRS in ribasim testmodels #1362

evetion opened this issue Apr 8, 2024 · 4 comments
Labels
tech-debt Improvements related to technical debt

Comments

@evetion
Copy link
Member

evetion commented Apr 8, 2024

#1339 added setting crs to our models, and made it mandatory to do so. Now all of our testmodels have EPSG:28992 set (the Dutch crs), but points are actually set with local coordinates like point(0,0) (https://github.com/Deltares/Ribasim/blob/main/python/ribasim_testmodels/ribasim_testmodels/allocation.py#L36).

Arguably, we need to set a dummy engineering crs for these models, like:

"""ENGCRS["Dummy CRS for testing",
    EDATUM["Unknown engineering datum"],
    CS[Cartesian,2],
        AXIS["X",unspecified,
            ORDER[1],
            LENGTHUNIT["unknown",0]],
        AXIS["Y",unspecified,
            ORDER[2],
            LENGTHUNIT["unknown",0]]]"""

#nitpick

@github-project-automation github-project-automation bot moved this to To do in Ribasim Apr 8, 2024
@evetion evetion added the tech-debt Improvements related to technical debt label Apr 8, 2024
@visr
Copy link
Member

visr commented Apr 8, 2024

There is no EPSG for this? I feel like having such a long code in the TOML that is often used as example is worse.
They aren't really unknown either, they are known to be some imaginary cartesian coordinate system.

@evetion
Copy link
Member Author

evetion commented Apr 8, 2024

There is no EPSG for this? I feel like having such a long code in the TOML that is often used as example is worse. They aren't really unknown either, they are known to be some imaginary cartesian coordinate system.

That's the engineering part of the datum, cartesian (x, y) in an imaginery space somewhere. There's no EPSG code for it, apart from some deprecated example ones (EPSG:5806 comes to mind)

I agree it's ugly, but I would prefer it over being wrong 😄. And this is a logical extension from the crs PR, or would you want to limit it to EPSG codes only?

@visr
Copy link
Member

visr commented Apr 8, 2024

I don't necessarily want to limit it to EPSG codes only, though we should stimulate people to use them when possible.

I agree it's ugly, but I would prefer it over being wrong

I prefer wrong, since wrong only affects plotting it on the map. We cannot plot it on any base map since the locations don't exist. Perhaps EPSG:5806 is fine to use? Strange that it is deprecated with no alternative.

@visr
Copy link
Member

visr commented May 3, 2024

Let's revisit when a suitable dummy EPSG is added ;)

@visr visr closed this as not planned Won't fix, can't repro, duplicate, stale May 3, 2024
@github-project-automation github-project-automation bot moved this from To do to ✅ Done in Ribasim May 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tech-debt Improvements related to technical debt
Projects
Archived in project
Development

No branches or pull requests

2 participants