Skip to content
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

Move to extension_metadata() instead of extension_field()? #218

Closed
kylebarron opened this issue Nov 3, 2023 · 3 comments · Fixed by #429
Closed

Move to extension_metadata() instead of extension_field()? #218

kylebarron opened this issue Nov 3, 2023 · 3 comments · Fixed by #429

Comments

@kylebarron
Copy link
Member

kylebarron commented Nov 3, 2023

It's not quite accurate to return extension_field() because that implies that you know the name of the field and its nullability. We need to expose both the data type and the extension metadata because arrow-rs doesn't have a DataType::Extension but it might be clearer to expose just those instead of wrapping them in a Field with an empty column name. So that would be:

fn data_type(&self) -> DataType;
/// Extension array name
fn extension_name(&self) -> str;
/// Extension array metadata
fn extension_metadata(&self) -> HashMap<str, str>;
@lewiszlw
Copy link
Contributor

lewiszlw commented Nov 3, 2023

Now extension metadata only contains extension name, is extension_metadata method still needed? If needed, I can add it.

@kylebarron
Copy link
Member Author

This crate hasn't added support yet for extension metadata. We will need to add support at some point for it to keep track of the column's coordinate system information, see https://geoarrow.org/extension-types#extension-metadata

@lewiszlw
Copy link
Contributor

Can you give a review on #224? I'd like to support ewkb after this merged.

kylebarron added a commit that referenced this issue Jan 12, 2024
Implementation of
https://github.com/geoarrow/geoarrow/blob/main/extension-types.md#extension-metadata

Follow ups:

- Don't include `ARROW:extension:metadata` key if neither `crs` nor
`edges` exists
- Allow python bindings to pass in CRS
- Pass on CRS in GeoParquet reader

Closes #218,
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants