Feat/wkb rewrite#45
Draft
wietzesuijker wants to merge 6 commits into
Draft
Conversation
* Import from apache arrow root level * Fix lint * Consolidate imports to use the main one
Replace @loaders.gl/wkt-based WKB parser with zero-dependency implementation modeled on geoarrow-rs. Direct DataView parsing, auto-detect type/dim from WKB headers, null handling via Arrow validity bitmap, round-trip encode/decode for all 6 OGC types. Bounds-checked against malformed input. ISO WKB + EWKB support. 63 tests: unit, round-trip, and geoarrow-data corpus.
Member
|
This is cool but not really reviewable as one big PR. If you want to work on this for geoarrow-js, I think we'd need to start with
There's also part of the TypeScript design that needs some thought. Like here const cap = capacity as PointCapacity;an effective type driven design should never have a cast like this. This can be managed by overloads, discriminated unions, etc. |
Author
|
Makes sense. Converting to draft. I opened an issue with the work plan, geoarrow-rs mapping, and a proposed fix for the type casts: #46. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Rewrites the WKB parser from scratch, replacing the
@loaders.gl/wktdependency with directDataViewparsing. Adds a WKB encoder (toWkb) for round-trip capability. Modeled on geoarrow-rs.Follows up on the review feedback in #44.
What changed
types.tsWkbTypeenum (1-6),Dimensionenum,coordSize()header.tscapacity.tsreader.tsparseWkb(data): two-pass (scan + fill), auto-detects type and dimensionswriter.tstoWkb(data): encodes GeoArrow arrays to ISO WKB (little-endian)What's new vs the old parser
@loaders.gl/wktand@loaders.gl/schemaremovedtoWkb(parseWkb(wkb))produces identical bytesRound-trip tested against geoarrow-data fixtures for all 6 types.
AI (Claude) supported my development of this PR.