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

Implement all applicable GeoInterface methods for DataFrames #70

Merged
merged 6 commits into from
Jun 20, 2024

Conversation

asinghvi17
Copy link
Contributor

But not traits.

This implements GI.crs and GI.geometrycolumns for DataFrames and DataFrameRows, as well as GI.getfeature for DataFrame and GI.geometry and GI.properties for DataFrameRows.

It also adds the namespace proposition from GeoInterface, so there are now four metadata entries in any GeoDataFrame by default. These are exact copies of the originals, and the original crs and geometrycolumns entries are retained for backwards compatibility.

The new metadata entries are GEOINTERFACE:crs and GEOINTERFACE:geometrycolumns, which are namespaced metadata according to the Arrow spec.

This does not support other tables at the moment - that's probably a decision best made in GeoInterface, since it would need to take on Tables.jl and DataAPI.jl as direct dependencies (to avoid bringing in Requires on Julia < 1.9).

@evetion
Copy link
Owner

evetion commented Jun 19, 2024

This seems lovely. Only drawback is that we make this repo the "blessed" geospatial dataframe implementation, since we do some type piracy on DataFrame.

@asinghvi17
Copy link
Contributor Author

asinghvi17 commented Jun 19, 2024

We could move this to GeoInterface (either by taking DataAPI as a direct dependency, or by a package extension (though that would rule out lower versions of Julia). This would allow generic metadata handling, but the cost is that DataFrames does become slightly "blessed", since a lot of other table formats don't handle metadata that well...

src/io.jl Outdated Show resolved Hide resolved
I'm not sure about this change, since it seems the `unknown_crs` thing used to work...
@evetion evetion merged commit 1b56097 into evetion:master Jun 20, 2024
10 of 12 checks passed
@asinghvi17 asinghvi17 deleted the as/more_geointerface branch June 20, 2024 11:55
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 this pull request may close these issues.

2 participants