-
Notifications
You must be signed in to change notification settings - Fork 56
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
Add optional "geometry_type" field per column #51
Add optional "geometry_type" field per column #51
Conversation
@jorisvandenbossche - what's needed to finalize this PR? It's still marked 'draft'. Would be great to get this in for 0.2.0. |
In this old version of "GeoParquet" (link) there is a similar approach for storing the types. In that case, it uses I think it's fine having both BTW, I miss the implementation of the field in the example. Let me know if you want me to add it. |
I think mostly some more feedback on what I actually added here / the open questions in #41 |
Ah, got it. I see - this currently does have it as an optional field and Unknown as an option. I do agree with your comment that we should pick one so there's not two ways to say unknown. I lean towards required and allow unknown. Follows flatgeobuf. |
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.
👏 Super happy to see this new field. Thank you so much @jorisvandenbossche and @rouault! I've faced this last week and a few minutes ago with multiple use cases where it's required.
Today: The tilers at CARTP need to guess the type of geometry, right now we need to add a query over the data of the user to determine the type of geometry (using samples and some heuristics... 😢 ). It's something not only for GeoParquet, it should be added as a feature in the data warehouses, i.e., given a geom column of a table have a way to get the geometry type from some metadata without exploring the data.
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 added some edits to make it so the field is required, but unknown is allowed.
I'm also ok if we make it optional and specify that leaving it out is the same as 'unknown', but prefer required & unknown option, to nudge implementors to thinking about it.
Co-authored-by: Chris Holmes <chomie@gmail.com>
Co-authored-by: Chris Holmes <chomie@gmail.com>
Co-authored-by: Chris Holmes <chomie@gmail.com>
As @jorisvandenbossche already agreed on the idea of required & unknown and @cholmes too, I'm merging. |
Closes #41