-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Arrow read/write: add GDAL specific field metadata #8631
Conversation
e942f7d
to
6eeffa5
Compare
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 think what you might want here is regular field metadata rather than an extension type? Regular field metadata might look like:
- Key:
GDAL:OGR:alternative_name
(with associated value) - Key:
GDAL:OGR:comment
(with associated value) - Key:
GDAL:OGR:default
(with associated value) - Key:
GDAL:OGR:subtype
(with associated value) - Key:
GDAL:OGR:width
(with associated value)
Rather than
- Key:
ARROW:extension:name
=gdal.field_metadata
- Key:
ARROW:extension:metadata
={"alternative_name": ... }
An extension type might perhaps be better suited to a GDAL type that could not be represented losslessly by an Arrow type and had to be serialized to a binary column in some way and given an extension name + metadata.
I think what you are trying to represent here is true "field metadata": the Arrow types are accurate and the values can be interpreted safely according to the type of the array...there's just some extra information for consumers who want it.
Arrow C++ silently drops unknown extension types fairly aggressively but I don't think that behaviour is guaranteed and might be different in another implementation (I might be alone but I personally wish it would warn or error by default in those cases to prevent misinterpreting true extension types).
…code path" This partially reverts commit fb92aa5. No longer needed since previous commit
6eeffa5
to
210a875
Compare
@paleolimbot Thanks for the hints! |
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.
Looks great!
CC @paleolimbot