milastdbx commented on code in PR #15834: URL: https://github.com/apache/iceberg/pull/15834#discussion_r3155139750
########## format/spec.md: ########## @@ -242,10 +242,12 @@ For `geometry` type, the CRS does not affect geometric calculations, which are a The default CRS value `OGC:CRS84` means that the objects must be stored in longitude, latitude based on the WGS84 datum. -Custom CRS values can be specified by a string of the format `type:identifier`, where `type` is one of the following values: - -* `srid`: [Spatial reference identifier](https://en.wikipedia.org/wiki/Spatial_reference_system#Identifier), `identifier` is the SRID itself. -* `projjson`: [PROJJSON](https://proj.org/en/stable/specifications/projjson.html), `identifier` is the name of a table property where the projjson string is stored. +Non-default CRS values are specified by any string that uniquely identifies a coordinate reference system associated with this type. +To maximize interoperability, suggested formats for CRS include, but are not limited to: +* `<projjson>` - A complete CRS definition embedded directly using the [PROJJSON](https://proj.org/en/stable/specifications/projjson.html) specification. Example for `OGC:CRS83`: `{"$schema": "https://proj.org/schemas/v0.7/projjson.schema.json","type": "GeographicCRS","name": "NAD83 (CRS83)","datum": {"type": "GeodeticReferenceFrame"...` Review Comment: I've updated the spec to reflect this. However something that bothers me: - `CRS values are specified by any string that uniquely identifies a coordinate reference system ` Here we are being very explicit that "ANY STRING" is accepted - `suggested formats for CRS include, but are not limited to` One more place where we emphasize that these are suggestions and not limitations Then at the end we do say that PROJJSON is forbidden (contradicts "ANY STRING" above). While it can be read that this is suggestions + blocklist, which is fine, i still want to make sure we are on same page. I guess the main question is - do we want for the above suggestions to actually be "limitations" according to spec ? Wdyt about latest wording ? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
