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

Update CRS reference #137

Closed
jyutzler opened this Issue Aug 26, 2015 · 12 comments

Comments

Projects
None yet
5 participants
@jyutzler
Contributor

jyutzler commented Aug 26, 2015

From @KRyden
The original OGC Geopackage specification was adopted prior to the adoption of the updated SRS WKT Specification OGC Well known text representation of coordinate reference systems. As a result, the original OGC Geopackage specification references older documents that should be superseded by the final version of OGC Well known text representation of coordinate reference systems, OGC 12-063r5, approved 13 August, 2014. Changes are required in the Geopackage specification document to adopt OGC 12-063r5, and eliminate ambiguities being encountered in the field from the older specification reference.

@jyutzler jyutzler added this to the tiles-corrigendum milestone Aug 26, 2015

jyutzler added a commit to jyutzler/geopackage that referenced this issue Aug 26, 2015

@jyutzler

This comment has been minimized.

Contributor

jyutzler commented Aug 26, 2015

@KRyden Please review this pull request #138 and let me know if it looks right to you. I just changed the reference # 32 - there is no reason to keep the old one around as it is only referenced in this section.

@pepijnve

This comment has been minimized.

Contributor

pepijnve commented Aug 26, 2015

@jyutzler this change risks breaking existing geopackage implementations. Everyone expects old style CRS WKT at the moment. If you switch to the new spec without a version bump, extension, etc. I would expect this to cause problems.
IIRC the last discussion I had about this with @pdaisey about this concluded that GeoPackage should wait for the upstream specs (i.e., SQL/MM and/or SF-SQL) to be updated first. Otherwise you GeoPackage becomes incompatible with SF-SQL as well.

@jyutzler

This comment has been minimized.

Contributor

jyutzler commented Aug 26, 2015

This would be part of 1.0.2 which is currently on hold until at least mid-September. GeoPackages are versioned and we believe the impact of this change is low. However, we have not made a final decision.

@KRyden

This comment has been minimized.

KRyden commented Aug 26, 2015

Pepijn is correct that there are multiple OGC specifications that need to update references to the CRS Well Known Text specification (12-063r5 adopted August 13, 2014). OGC management will need to help move that effort, and with all of these documents on different revision cycles (some stretching out years), timing becomes a real problem when you need to fix things and get it right. We need to get this moving, and Geopackage is in a good position to start driving it forward as it is directly impacted. We should update Simple Features ASAP.

Regarding compatibility, section 6.7 of 12-063r5 addresses the expectations for compatibility - "Backward compatibility means that an implementation of the text strings in this International Standard would be able to read CRS WKT strings conforming to the old (ISO 19125-1:2004) syntax" - so newer parser implementations should be handling older strings. This is not just an issue for existing geopackages - all software supporting interpretation of CRS WKT strings needs to make this move. What's going to be an issue in the field is older software that does not know how to process newer CRS WKT strings - each software provider will need to address that issue in their own way. At some point, we have to move on.

Jeff - Any valid CRS WKT should be sufficient - the following statement is probably a result of referencing 01-009 which is superseded, and should be removed:

"Values SHALL include optional EBNF entities. Values for SRS other than WGS-84 SHOULD include optional EBNF entities. Values MAY omit optional and EBNF entities."

And the braces "<>" should be removed from around in this sentence:
EBNF name and number values MAY be obtained from any specified , e.g. <<13>><<14>>.

So the final paragraph would read:
Definition column WKT values in the gpkg_spatial_ref_sys table SHALL define the Spatial Reference Systems used by feature geometries and tile images, unless these SRS are unknown and therefore undefined as specified in <<_requirement-11>>. Values SHALL be constructed and applications SHALL interpret the EBNF using rules in <<32>>. EBNF name and number values MAY be obtained from any specified authority, e.g. <<13>><<14>>. For example, see the return value in <<spatial_ref_sys_data_values_default>> Test Method step (3) used to test the definition for WGS-84 per <<_requirement-11>>:

jyutzler added a commit to jyutzler/geopackage that referenced this issue Aug 26, 2015

jyutzler added a commit to jyutzler/geopackage that referenced this issue Aug 26, 2015

@jyutzler

This comment has been minimized.

Contributor

jyutzler commented Aug 26, 2015

@KRyden I updated the PR based on your feedback. Pardon my sloppiness, this is the correct commit:
jyutzler@473ebba

jyutzler added a commit to jyutzler/geopackage that referenced this issue Aug 26, 2015

@pepijnve

This comment has been minimized.

Contributor

pepijnve commented Aug 26, 2015

@jyutzler they're not quite versioned yet. The only file wide version number is the application id, but that's optional. You'll need to make sure to bump that value in 1.02 then and perhaps make it mandatory. Still feels like a pretty big change for a point release to me...

@jyutzler

This comment has been minimized.

Contributor

jyutzler commented Aug 26, 2015

Yes, the app ID will get bumped to 1.0.2 this time around.

@rouault

This comment has been minimized.

Contributor

rouault commented Aug 31, 2015

Woo, that's a significant change ! I agree with @pepijnve . Something like that would be worth at least worth a 1.1 number IMHO. Especially if you change the application_id. That's no longer a corrigendum. It's like WFS 1.1 finally stating that EPSG axis order should be enforced.

FYI, regarding GDAL full uptake of this, I would expect several weeks of work needed to implement WKT 2.0 support, and especially figuring out how to make that live with WKT 1.0 that is expected by the rest of the codebase. That said, without changing it, on reading of GPKG databases, if there's an EPSG code attached to the SRS, then the 'definition' of the gpkg_spatial_ref_sys is ignored, and the SRS is resolved from GDAL EPSG database, so even with WKT 2.0 defs, there could be some compatibility.

@thomasneirynck

This comment has been minimized.

Contributor

thomasneirynck commented Sep 1, 2015

I broadly agree with @pepijnve and @rouault. This feels like a big change, due to the risk for compatibility issues. application id is still 1.0 right now? Bumping it to 1.0.2 may help, but few implementers have a habit to assume that a patch increase (ie. 2) would affect compatibility IMO.

I am personally not familiar enough with the changes to WKT to gauge how much work it would be though to make this change.

@jyutzler

This comment has been minimized.

Contributor

jyutzler commented Sep 1, 2015

The SWG has reached a decision today.

  • It is not appropriate to make such a substantive change to the core specification at this time. Therefore #138 will not be accepted as written.
  • As @KRyden reports, there are instances of compliant GeoPackages not being interoperable due to defects in WKT 1.0 as described in 01-009. Therefore it is in the best interests of the community to adopt 12-063 (WKT 2.0) sooner rather than later. We acknowledge that funding and release cycles make this problematic.
  • We will produce an extension to GeoPackage that specifies adoption of 12-063. This extension will be incorporated into the tiles corrigendum. (It will concurrently be incorporated into the US National System for GeoINT profile for GeoPackage.) This extension will provide a way ahead for those who are ready for such a thing without impacting everyone else.
  • We will engage with the OGC architecture committee to ensure that there is a plan for adopting 12-063 across all relevant standards including the Simple Features Profile.
  • The SWG remains interested in incorporating the new extension into the core at some point, but there is no timetable for doing so.
@pepijnve

This comment has been minimized.

Contributor

pepijnve commented Sep 1, 2015

@trgn one of the really big changes is that TOWGS84 is no longer supported. It was removed for a good reason, but its removal does have some practical implications. Software that implements an early bound, hub-and-spoke with WGS84 as hub, coordinate transformation engine will be impacted most since they'll have to obtain the datum transformation parameters from some other source. As @rouault said, if the authority information is available you can always use that to do lookups in an internal database, but of course there's always going to be the case where authority information is not provided (or refers to an authority that is unknown to the client software). If you encounter one of those I guess you either have to try to do reverse lookups in the EPSG (or some other) database or ask the user for input.

@jyutzler

This comment has been minimized.

Contributor

jyutzler commented Nov 10, 2015

This ticket will remain open until the extension is authored.

jyutzler added a commit to jyutzler/geopackage that referenced this issue Nov 26, 2015

jyutzler added a commit that referenced this issue Nov 26, 2015

Merge pull request #168 from jyutzler/re
#137 new extension for CRS WKT

@jyutzler jyutzler modified the milestones: tiles-corrigendum, 1.0.2 Nov 29, 2015

@jyutzler jyutzler closed this Nov 29, 2015

This was referenced Dec 27, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment