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

GeoJSON driver lco RFC7946 should be YES #4010

Closed
Joonalai opened this issue Jun 18, 2021 · 9 comments
Closed

GeoJSON driver lco RFC7946 should be YES #4010

Joonalai opened this issue Jun 18, 2021 · 9 comments

Comments

@Joonalai
Copy link

Expected behavior and actual behavior.

GeoJSON driver uses obsolete GeoJSON 2008 standard by default with RFC7946 default value being NO. Since GeoJSON 2008 standard is marked as obsolete and it redirects to newer RFC7946 standard, the default value of RFC7946 should be 'YES'.

Additional context

This issue relates to this QGIS issue.

@rouault
Copy link
Member

rouault commented Jun 18, 2021

I'm not so keen on using RFC7946 by default. Doing so imply that we reproject to EPSG:4326 which might not always be suitable.

@jratike80
Copy link
Collaborator

I agree with Even and just because of the CRS. But what bugs me is that I never remember RFC7946 without having a look at the documentation.

@Joonalai
Copy link
Author

That is a fair point, I agree. What do you think about the default COORDINATE_PRECISION of 15 digits? @tjukanovt has started a conversation about that on twitter.

@jratike80
Copy link
Collaborator

Coordinate precision applies also to other text bases formats (GML, CSV). Binary formats usually use doubles for storing coordinates and inaccuracy of floating points tend to lead loads of decimal (decimal 10.1 becomes 10.1000003814697265625).

I am not sure what to say about the default precision. Degree units with 5 decimals may be enough for visualizing but when the aim is to deliver then I think that there should be as little data loss as possible. Differences in coordinates at nanometer level make them still not equal for computers. GeoJSON is a somewhat lossy format but it is still used as a medium for delivering data.

With 7 decimals the precision corresponds to about 1 cm. That's usually good enough for visualizing but it is likely that some coordinates in a typical GIS data would not survive a roundtrip like PostGIS - GeoJSON - PostGIS.

Every now and then I think that defaults are evil and it would be better to force users to give exact parameters and make them to take the responsibility but most of the time I understand that it would be cruel.

So what is your suggestion for making everyone happy?

@tjukanovt
Copy link

My 2 cents as I opened the original issue on the QGIS side:

I think as we are talking about GeoJSON and other geospatial formats, it is difficult to find reasoning to use precision beyond 1 cm in 99 % of use cases. Of course there are the ~1% of use cases where the decimals beyond 7 do matter, but I think the formats be default should serve the 99% of cases really well rather than having the 99% suffer in performance because of the 1%.

What is the downside of having all of the decimals included in GeoJSONs? Quite often GeoJSON is used on the web and having less JSON to parse with can speed up applications significantly. Leaflet, OpenLayers, kepler.gl all happily use GeoJSON and all of them obviously have better performance if there is less data to chew. Also in cases of data transfer, does it make sense to include the data by default if it is commonly known it is inaccurate and not useful?

Should there be a flag to indicate if the data should include all the decimals and by default the decimals should be limited to 7? So in general this would help to reduce file sizes.

These are just my thoughts 🤷🏼‍♂️

@jratike80
Copy link
Collaborator

I may agree that PRECISION=7 could be a a better default for GeoJSON(2008) but why not to think a bit deeper by the same? Even precision=7 gives unreasonable many decimals when the data are in some projected coordinate system. Perhaps a better option would be to implement a selection PROFILE=WEB/DATA into QGIS and use WEB as default. That would yield PRECISION=7 with geographic CS and PRECISION=2 when crs has metre or foot as unit.

I thought that users could avoid having so many decimals also by cleaning their data by rounding but that does not seem to work with GeoJSON.

ogr2ogr -f geojson -dialect sqlite -sql "select geomfromtext('POINT (538858.65 6740202.117)') as geom"  jsontest.json foo.shp
Result:
"geometry": { "type": "Point", "coordinates": [ 538858.650000000023283, 6740202.11699999962002 ] }

With GML the result is as I assumed:

ogr2ogr -f gml -dialect sqlite -sql "select geomfromtext('POINT (538858.65 6740202.117)') as geom"  jsontest2.gml jsontest1.json
Result:
<gml:Point><gml:coordinates>538858.65,6740202.117</gml:coordinates></gml:Point>

Another place to think deeper is to consider if all text based formats could handle the coordinate precision in the same way and by using same parameters. Now we have COORDINATE_PRECISION for GeoJSON, config option OGR_WKT_PRECISION for CSV, DECIMAL_PRECISION and SIGNIFICANT_DIGITS for geojsonseq (and aaigrid and xyz raster formats), and nothing at all for GML.

@Joonalai
Copy link
Author

Another place to think deeper is to consider if all text based formats could handle the coordinate precision in the same way and by using same parameters. Now we have COORDINATE_PRECISION for GeoJSON, config option OGR_WKT_PRECISION for CSV, DECIMAL_PRECISION and SIGNIFICANT_DIGITS for geojsonseq (and aaigrid and xyz raster formats), and nothing at all for GML.

I think this idea is worth considering but maybe this issue is not a right place, if GeoJSON 2008 is going to be the default value for now. @jratike80 should a another issue be made for that and this one be closed?

@jratike80
Copy link
Collaborator

This ticket is about if -lco RFC7946 should default to YES or NO in GDAL and we seem to have a consensus that the current behavior is OK. So closing feels appropriate.

@pathmapper
Copy link
Contributor

FWIW, if this ever comes to debate again, I'm +1 for

-lco RFC7946 should default to YES

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

5 participants