-
Notifications
You must be signed in to change notification settings - Fork 4
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
feat(geoarrow-types): pyarrow geometry extension type implementation #51
Conversation
paleolimbot
commented
Jun 13, 2024
•
edited
Loading
edited
UNSPECIFIED = -1 | ||
"""Unspecified dimensions""" | ||
|
||
UNKNOWN = 0 | ||
"""Unknown or mixed dimensions""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not 100% clear on the difference between unspecified and unknown
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The intention is that it's like GEOMETRY (i.e., potentially mixed but we don't know) but for dimensions. I added it here because I realized that gt.wkb().to_pyarrow().dimensions
was returning UNSPECIFIED
, which is not really an accurate description of what's happening.
It could be argued that UNSPECIFIED should just be None
, and it could be implemented that way (but makes the typing slightly more awkward). In the case of crs
, None
is a perfectly acceptable value, so there at least needs to be an UNSPECIFIED
sentinel for that one (such that coalesce(None, crs.UNSPECIFIED
-> None
).
The intention is that users don't run into UNSPECIFIED all that often (they just generate it by implicitly not adding a type constraint).
import geoarrow.types as gt
gt.TypeSpec.coalesce(gt.GeometryType.UNSPECIFIED, gt.GeometryType.POINT)
#> TypeSpec(GeometryType.POINT)
gt.TypeSpec.coalesce(gt.GeometryType.GEOMETRY, gt.GeometryType.POINT)
#> TypeSpec(GeometryType.GEOMETRY)
gt.TypeSpec.coalesce(gt.Dimensions.UNSPECIFIED, gt.Dimensions.XYZ)
#> TypeSpec(Dimensions.XYZ)
gt.TypeSpec.coalesce(gt.Dimensions.UNKNOWN, gt.Dimensions.XYZ)
#> TypeSpec(Dimensions.UNKNOWN)
gt.TypeSpec.common(gt.Dimensions.XYM, gt.Dimensions.XYZ)
#> TypeSpec(Dimensions.XYZM)
gt.TypeSpec.common(gt.Dimensions.UNSPECIFIED, gt.Dimensions.XYZ)
#> TypeSpec(Dimensions.XYZ)
gt.TypeSpec.common(gt.Dimensions.UNKNOWN, gt.Dimensions.XYZ)
#> TypeSpec(Dimensions.UNKNOWN)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As an aside, I do like the ideas of coalesce
and common
for types. I had so much pain working with type casting in geoarrow-rs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I stole the idea from the R vctrs package! https://vctrs.r-lib.org/reference/internal-faq-ptype2-identity.html