Use hexWKB in JSON diff/show output #71

craigds · 2020-05-04T04:35:34Z

This replaces GeoJSON geometries with (little-endian) hex-encoded WKB
in show --json and diff --json commands. --geojson output is
unaffected.

This:

reduces patch/diff size by quite a lot, especially for large
geometries
makes patch generation/consumption more efficient; no pass via OGR is
required in most circumstances.
improves support for exotic geometries (GeoJSON doesn't support
curved geometry types)

fixes #62

Size of output

# points, with GeoJSON
$ sno show --json | wc -c
5405


# points, with hexWKB:
$ sno show --json | wc -c
4205

If you combine this with --json-style=extracompact from #70:

# points, with GeoJSON
$ sno show --json --json-style=extracompact | wc -c
2576


# points, with hexWKB:
$ sno show --json --json-style=extracompact | wc -c
2316

rcoup

sno/gpkg.py

tests/data/patches/points-1U-1D-1I.snopatch

sno/gpkg.py

tests/test_diff.py

sno/diff_output.py

rcoup · 2020-05-05T07:56:59Z

sno/diff_output.py

-            g = gpkg.geom_to_ogr(v)
-            f["geometry"] = json.loads(g.ExportToJson())
+            g = gpkg.gpkg_geom_to_ogr(v)
+            f['geometry'] = json.loads(g.ExportToJson())


OT: I feel like I've written this code 1000 times. Maybe it's time to fix this in OGR?

(not right this second)

ie some kind of ogr.ExportToDeserializedJson() method?

One problem is that different bindings would have to implement it differently. I'm completely ignorant of how Java handles deserialized JSON but I'm not sure I want to know either...

In principle, yes (ExportToJsonStruct()?). It's possible to add language-specific methods in SWIG, so could ignore Java in the meantime.

sno/gpkg.py

tests/test_gpkg.py

sno/diff_output.py

This replaces GeoJSON geometries with (little-endian) hex-encoded WKB in `show --json` and `diff --json` commands. `--geojson` output is unaffected. This: * reduces patch/diff size by quite a lot, especially for large geometries * makes patch generation/consumption more efficient; no pass via OGR is required in most circumstances. * improves support for exotic geometries (GeoJSON doesn't support curved geometry types) fixes #62

Including endianness and envelopes

They're not GeoJSON anymore anyway, because we're using hexWKB geometries, so best not to pretend they are.

Supports curves and POINT EMPTY

rcoup · 2020-05-07T07:55:30Z

Can you summarize/include/link examples of whatever formats we have finalised on?

rcoup · 2020-05-07T07:56:27Z

sno/diff-view.html

@@ -125,7 +125,7 @@
            }).addTo(map)

            var layerGroup = L.featureGroup()
-            for (let [dataset, diff] of Object.entries(DATA['sno.diff/v1'])) {
+            for (let [dataset, diff] of Object.entries(DATA['sno.diff/v1+hexwkb'])) {


how's it going to view wkb in the browser? Surely needs the GeoJSON output?

I did this by accident, so good catch. I'm surprised this is using the json diff and not the geojson diff. That throws a spanner in the works a bit, because I can't just switch tot he geojson one, it's quite different. Will attempt to though.

It's using the JSON because the geojson ends up in multiple files

Alternatively, it could serialise all the geojson into an array in the HTML or something. No problem doing that, but it needs to work :)

The --geojson diff doesn't actually differentiate multiple datasets, so it's not enough for this.

So I guess to make this work, I need to add a format variant for --json diffs with GeoJSON geometries?

Don’t understand... the geojson gets written to multiple files in a directory.

aha, so it does (if you have >1 dataset). I just saw it with one dataset and got lost on how the different ones were going to be differentiated, but didn't realise it used multiple files

otherwise there's no meaningful way to view them in QGIS/etc

fixed in cb28c3b - it now writes geojson files to a temp dir and then reads them back in again. Could have done it some other way, e.g. by passing an empty dict to resolve_object_path and making it create a StringIO for each dataset, but it felt like a Weird Hack so I didn't

tests/test_diff.py

Possibly commented out by accident in b5f7c7c

Generate it from the `geojson` diff rather than the `json` one, because the `json` one now uses hexWKB geometries. Because the layout's a bit different, this means the JS in the HTML template needs to do a bit more work, but it's quite achievable. Yay for ES7

Since #71 we haven't used GeoJSON for diffs or patches, however we still have some GeoJSON artifacts that are now unnecessary and unused. This moves all feature attributes into the feature object, and removes the `properties`, `geometry` and `id` keys.

craigds requested review from olsen232 and rcoup and removed request for olsen232 May 4, 2020 04:35

rcoup requested changes May 4, 2020

View reviewed changes

craigds force-pushed the hex-wkb-geometries branch from 8addc82 to 7807bcf Compare May 5, 2020 02:21

rcoup reviewed May 5, 2020

View reviewed changes

craigds requested a review from rcoup May 6, 2020 02:14

craigds added 5 commits May 7, 2020 17:07

Rename sno.gpkg geometry conversion functions

fd4ae27

More comprehensive tests for GPKG geom conversion

95b103a

Including endianness and envelopes

Drop 'type' key from json-serialized rows

0399a40

They're not GeoJSON anymore anyway, because we're using hexWKB geometries, so best not to pretend they are.

Fix POINT EMPTY ending up as POINT(0 0)

704b4c8

craigds force-pushed the hex-wkb-geometries branch from c09db8f to af1fc02 Compare May 7, 2020 05:07

craigds added 3 commits May 7, 2020 19:20

Diffs: Use "sno.diff/v1+hexwkb"

b99e0ef

Use ISO WKB instead of OGC WKB

ba53ebe

Supports curves and POINT EMPTY

Comments and review tweaks

304c827

craigds force-pushed the hex-wkb-geometries branch from af1fc02 to 304c827 Compare May 7, 2020 07:20

rcoup reviewed May 7, 2020

View reviewed changes

craigds added 2 commits May 7, 2020 20:35

Reinstate test-diff asserts

cec3a13

Possibly commented out by accident in b5f7c7c

Fix HTML diff output

cb28c3b

Generate it from the `geojson` diff rather than the `json` one, because the `json` one now uses hexWKB geometries. Because the layout's a bit different, this means the JS in the HTML template needs to do a bit more work, but it's quite achievable. Yay for ES7

craigds requested a review from rcoup May 7, 2020 20:50

olsen232 approved these changes May 7, 2020

View reviewed changes

craigds merged commit 25fa954 into master May 7, 2020

craigds deleted the hex-wkb-geometries branch May 7, 2020 21:42

craigds mentioned this pull request Jun 13, 2020

diff/show/apply: Simplify JSON diffs/patches #117

Merged

craigds mentioned this pull request Apr 21, 2021

Improvements to streaming JSON consumption #412

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use hexWKB in JSON diff/show output #71

Use hexWKB in JSON diff/show output #71

craigds commented May 4, 2020

rcoup left a comment

rcoup May 5, 2020

craigds May 5, 2020 •

edited

Loading

rcoup May 6, 2020

rcoup commented May 7, 2020

rcoup May 7, 2020

craigds May 7, 2020

rcoup May 7, 2020

rcoup May 7, 2020

craigds May 7, 2020

rcoup May 7, 2020 •

edited

Loading

craigds May 7, 2020

rcoup May 7, 2020

craigds May 7, 2020

Use hexWKB in JSON diff/show output #71

Use hexWKB in JSON diff/show output #71

Conversation

craigds commented May 4, 2020

Size of output

rcoup left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

craigds May 5, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rcoup commented May 7, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rcoup May 7, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

craigds May 5, 2020 •

edited

Loading

rcoup May 7, 2020 •

edited

Loading