moomindani opened a new issue, #3975:
URL: https://github.com/apache/iceberg-python/issues/3975
`mkdocs/docs/geospatial.md` documents a deliberate choice:
> **WKB/WKT Conversion**: Converting between WKB bytes and WKT strings
requires external libraries (like Shapely). PyIceberg does not include this
conversion to avoid heavy dependencies.
I would like to ask whether that is still the position, because it has a
consequence I am not sure was intended. `pyiceberg/conversions.py` leaves four
stubs raising `NotImplementedError` — `to_json` and `from_json` for
`GeometryType` and `GeographyType` — and these are reached from metadata
handling rather than only from value conversion. `from_json` is called by
`_deserialize_default_value` (`pyiceberg/types.py:377`) and `to_json` by
`NestedField.serialize_model` (`pyiceberg/types.py:458`), so a geometry or
geography column that declares `initial-default` or `write-default` cannot have
its schema parsed or serialized:
```python
Schema.model_validate({"type": "struct", "schema-id": 0, "fields": [
{"id": 1, "name": "geom", "required": False, "type": "geometry",
"initial-default": "POINT (1 2)"}]})
# NotImplementedError: Geometry JSON deserialization requires WKT to WKB
conversion, ...
```
Serializing a field that already holds WKB defaults fails symmetrically, as
a `PydanticSerializationError` wrapping the same error. (`from_json` does pass
`bytes` through unchanged; it is the WKT string form the spec actually mandates
in JSON that fails.)
The options I can see, and what I think each costs:
1. **A WKB/WKT codec inside PyIceberg.** No new dependency, and conversion
stays available wherever metadata is read. But Appendix G binds the format to
OGC Simple Features, ISO variants included, so conforming means all geometry
classes including `GeometryCollection`, `EMPTY` forms including WKB's NaN empty
point, XY/XYZ/XYM/XYZM, and both endiannesses, with input validation. PyIceberg
parses WKB nowhere today, so this is a real component, not a helper — larger
than I first assumed, which is why I am asking rather than opening a PR.
2. **Reuse the existing `geoarrow` extra.** No new dependency name, and it
already provides the conversions. The awkward part is that `geoarrow` currently
degrades gracefully — `visit_geometry` falls back to `large_binary` when it is
absent (`pyiceberg/io/pyarrow.py`) — whereas here the same schema would parse
or fail depending on the install profile. I could find no precedent for a core
path requiring an extra.
3. **`shapely` as an optional extra**, which is what the documentation above
declined.
4. **Keep the current position** and treat geospatial defaults as
unsupported, but make that explicit — the failure currently surfaces as a
`NotImplementedError` from inside `Schema.model_validate`, which is hard to act
on.
I do not have a strong preference and I am happy to implement whichever you
would prefer, including 4. What I would rather not do is pick the dependency
policy myself.
Related: #1818 lists Geometry/Geography as an open item but links #1820,
which was closed once #2859 landed the types and the binary and Parquet paths;
these stubs are what remained. #3004 and #3067 were both closed while stale and
neither touched `conversions.py`.
--
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]
