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

Allow epsg-code to be provided as string? #259

Closed
m-mohr opened this issue Nov 25, 2021 · 10 comments
Closed

Allow epsg-code to be provided as string? #259

m-mohr opened this issue Nov 25, 2021 · 10 comments

Comments

@m-mohr
Copy link
Member

m-mohr commented Nov 25, 2021

The specification asks to provide EPSG codes as numbers (e.g. 4326 instead of EPSG:4326). We see many code snippets that actually use the string variant, which strictly speaking should be WKT2.

So an idea coming up in recent discussions is that we use the clients to correct this common mistake.
So for subtype epsg-code a client could simply match against the pattern/regexp /^EPSG:(\d{4,})$/i and replace it with the first match converted to an int.

In pseudo-code something like:

if $subtype == 'epsg-code':
  $matches = $value.match(/^EPSG:(\d{4,})$/i)
  if $matches:
    $value = int($matches[1])
@m-mohr m-mohr changed the title Allow epsg-code be provided as string? Allow epsg-code to be provided as string? Nov 25, 2021
@clausmichele
Copy link
Member

The Equi7 requires the following string to be parsed for using it in load_collection
https://discuss.eodc.eu/t/get-specific-data-extent-using-equi7-coordinates/223
"crs": "PROJCRS[\"Azimuthal_Equidistant\",BASEGEOGCRS[\"WGS 84\",DATUM[\"World Geodetic System 1984\",ELLIPSOID[\"WGS 84\",6378137,298.257223563,LENGTHUNIT[\"metre\",1]]],PRIMEM[\"Greenwich\",0,ANGLEUNIT[\"degree\",0.0174532925199433]],ID[\"EPSG\",4326]],CONVERSION[\"Modified Azimuthal Equidistant\",METHOD[\"Modified Azimuthal Equidistant\",ID[\"EPSG\",9832]],PARAMETER[\"Latitude of natural origin\",53,ANGLEUNIT[\"degree\",0.0174532925199433],ID[\"EPSG\",8801]],PARAMETER[\"Longitude of natural origin\",24,ANGLEUNIT[\"degree\",0.0174532925199433],ID[\"EPSG\",8802]],PARAMETER[\"False easting\",5837287.81977,LENGTHUNIT[\"metre\",1],ID[\"EPSG\",8806]],PARAMETER[\"False northing\",2121415.69617,LENGTHUNIT[\"metre\",1],ID[\"EPSG\",8807]]],CS[Cartesian,2],AXIS[\"easting\",east,ORDER[1],LENGTHUNIT[\"metre\",1,ID[\"EPSG\",9001]]],AXIS[\"northing\",north,ORDER[2],LENGTHUNIT[\"metre\",1,ID[\"EPSG\",9001]]]]"

@m-mohr
Copy link
Member Author

m-mohr commented Feb 9, 2022

Is that a client or a server issue, @clausmichele ?

@clausmichele
Copy link
Member

Well, the question is: how does the client understand if the provided value is an integer referring to a crs code, an epsg code string or a WKT string?

@m-mohr
Copy link
Member Author

m-mohr commented Feb 9, 2022

Does the client need to understand it? Usually, it just needs to be passed through to the server, no?

@clausmichele
Copy link
Member

Yes, but from this issue https://github.com/openEOPlatform/architecture-docs/issues/159#issuecomment-978126164 it seemed that the plan was to convert an EPSG string to the integer representing it client side, since the API requires a crs to be specified as an integer.

@m-mohr
Copy link
Member Author

m-mohr commented Feb 10, 2022

Oh, I see... well you can easily distinguish them, see the pseudo-code below for an example:

if (val is string)
  if (val.matches(/^epsg:(\d{4,})$/i)) -> EPSG (first match of the regexp)
  else if (val.trim().startsWith('+')) -> Proj4
  else -> likely WKT
else if (val is integer) -> EPSG
else if (val is object/dict) -> PROJJSON
else -> invalid

or just do the following to convert epsg strings to int and pass the rest through:

if (val is string and (match = val.matches(/^epsg:(\d{4,})$/i))) 
  return Integer.parseString(match[1])
else
  return val

@m-mohr
Copy link
Member Author

m-mohr commented Mar 10, 2023

It would be great to get this small convenience into the Python client as all other clients already handle it smoothly:
Open-EO/openeo-processes#391 (comment)

@jdries
Copy link
Collaborator

jdries commented Aug 8, 2023

merged

@jdries jdries closed this as completed Aug 8, 2023
@soxofaan
Copy link
Member

soxofaan commented Aug 8, 2023

Reopening this issue: merged PR #453 introduced a new runtime dependency pyproj. It was already an (indirect) test dependency so tests didn't flag this.

However, pyproj is not a pure python lib, for which we have to be extra careful to add as runtime dependency.

I think we can extend PR #453 to make this an optional dependency

@soxofaan soxofaan reopened this Aug 8, 2023
soxofaan added a commit that referenced this issue Aug 9, 2023
- rename to `normalize_crs` because it is not only about EPSG output, WKT2 output is also allowed
- reworked and simplified pyproj availability handling: when available: use it fully, when not: do best effort normalization
- Move tests into class for better grouping and overview
- add test coverage for "no pyproj available" code path
@soxofaan
Copy link
Member

soxofaan commented Aug 9, 2023

resolved now, issue can be closed again

@soxofaan soxofaan closed this as completed Aug 9, 2023
soxofaan added a commit that referenced this issue Aug 9, 2023
Skip based on pyproj version iso python version
(allows testing the skips on higher python versions too)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants