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

ESRI morphing of datum names even when not asking for ESRI dialect WKT #1835

Closed
ravenAtSafe opened this issue Jan 9, 2020 · 10 comments
Closed

ESRI morphing of datum names even when not asking for ESRI dialect WKT #1835

ravenAtSafe opened this issue Jan 9, 2020 · 10 comments

Comments

@ravenAtSafe
Copy link

@ravenAtSafe ravenAtSafe commented Jan 9, 2020

l_name = io::WKTFormatter::morphNameToESRI(l_name);

Is it correct to ask for "ESRI morphing" of datum names in the location identified above? (Still looking for a concrete case of harm, discovered due to us defining our own coordinate systems via GDAL.)

@rouault
Copy link
Member

@rouault rouault commented Jan 9, 2020

That's what GDAL < 3 has always done when exporting WKT1, and as this is the WKT1_GDAL code path, this attempts to replicate as much as possible what GDAL < 3 did, be it questionable...
If we had a WKT1_EPSG / WKT1_COSHER option, then we might not want to do that indeed.

@TheDorkKnight
Copy link

@TheDorkKnight TheDorkKnight commented Jan 10, 2020

Are you referring to OGR_SRSNode::MakeValueSafe()? In GDAL 2.4.1 I searched for any usages

jake@my-machine:/my/gdal/dir$ grep -nr "MakeValueSafe" .
./ogr/ogr_srsnode.cpp:750:/*                           MakeValueSafe()                            */
./ogr/ogr_srsnode.cpp:760:void OGR_SRSNode::MakeValueSafe()
./ogr/ogr_srsnode.cpp:768:        GetChild(iChild)->MakeValueSafe();
./ogr/ogr_spatialref.h:105:    void        MakeValueSafe();

and there weren't any real callers.

@rouault
Copy link
Member

@rouault rouault commented Jan 10, 2020

Are you referring to OGR_SRSNode::MakeValueSafe()

The "massaging" was done in ogr/ogr_fromepsg.cpp by the OGREPSGDatumNameMassage() function. If you try gdalsrsinfo EPSG:xxxx of GDAL 2.4 and projinfo EPSG:xxxx -o WKT1_GDAL, you'll see that the outputs should be mostly identical (except differences on TOWGS84[])

@TheDorkKnight
Copy link

@TheDorkKnight TheDorkKnight commented Jan 10, 2020

Ah, I see. The difference then is that in GDAL 2.4 you could create a geographic coordsys with some method other than SetEPSGGeo[gc]CS and the datum names would not be "massaged", but now all datum names will be modified as they pass through PROJ.

@rouault
Copy link
Member

@rouault rouault commented Jan 10, 2020

yes that's slightly different, and not easy to completely replicate GDAL < 3 behaviour. In GDAL < 3, the OGRSpatialReference was just a representation of a WKT1 object, and could only handle that. In PROJ 6, WKT flavors are just import/export formats, and we morph everything to ISO-19111 compliant objects by using EPSG compliant names as much as possible. Thus the only way to format WKT1:GDAL from EPSG definitions as it was done in GDAL 2 is to do the datum name massaging at export to WKT1.
Is that a problem ? (not sure I'd have a solution...)

@ravenAtSafe
Copy link
Author

@ravenAtSafe ravenAtSafe commented Jan 10, 2020

It might not be a problem beyond being "different and therefore scary". We'll consider further. Thanks for the info!

@TheDorkKnight
Copy link

@TheDorkKnight TheDorkKnight commented Jan 10, 2020

Is that a problem ?

Well it's a little lossy and ugly, but not really harmful for EPSG coordinate systems. The difficulty occurs when trying to do a round trip from WKT -> OGRSpatialReference -> WKT where the datum names come from a non-EPSG authority (which is slightly niche to be sure, but happens).

One could get a situation where it was possible to recognize a datum in GDAL 2.4 but not in GDAL 3.0, such as:

DATUM["MyDatum_#1(2)", ... and
DATUM["MyDatum_(1.2)", ...

which would both be massaged to:
DATUM["MyDatum__1_2_",...
in GDAL 3 / PROJ 6

In practice this is probably unlikely, but it's not uncommon for applications to allow users to specify their own WKT.

@TheDorkKnight
Copy link

@TheDorkKnight TheDorkKnight commented Jan 10, 2020

rouault added a commit that referenced this issue Jan 11, 2020
WKT1_GDAL export: limit datum name massaging to names matching EPSG (fixes #1835)
@ravenAtSafe
Copy link
Author

@ravenAtSafe ravenAtSafe commented Jan 14, 2020

Thank you for the patch @rouault - we have confirmed that it restores the GDAL behavior that we want. Are you pretty confident it will stay in? If you are we'll apply the patch locally until the next PROJ release.

@rouault
Copy link
Member

@rouault rouault commented Jan 14, 2020

Are you pretty confident it will stay in?

unless someone would point out it badly breaks important use cases, I don't see why it wouldn't stay.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants