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

Add support for GI.crs #24

Closed
eliascarv opened this issue Oct 3, 2024 · 7 comments · Fixed by #25
Closed

Add support for GI.crs #24

eliascarv opened this issue Oct 3, 2024 · 7 comments · Fixed by #25

Comments

@eliascarv
Copy link
Contributor

The GeoParquet.read function returns a DataFrame, which I imagine is what prevents the GI.crs function from being implemented, as that would be type piracy. Perhaps returning a wrapper type that implements the Tables.jl interface would be a viable solution?

Here is an example of a file that has CRS, but is not returned by the GI.crs function:

julia> using GeoParquet

julia> using Parquet2

julia> using JSON3

julia> import GeoInterface as GI

julia> df = GeoParquet.read("example.parquet")
5×6 DataFrame
 Row │ pop_est         continent      name                      iso_a3   gdp_md_est  geometry                          
     │ Float64?        String?        String?                   String?  Int64?      WellKnow                         
─────┼─────────────────────────────────────────────────────────────────────────────────────────────────────────────────
   1889953.0        Oceania        Fiji                      FJI            5496  WellKnownBinary{Geom, Vector{UIn
                                                                                     
                                                                                                         4 rows omitted

julia> GI.crs(df)

julia> ds = Parquet2.Dataset("example.parquet");

julia> meta = Parquet2.metadata(ds)["geo"];

julia> json = JSON3.read(meta)
JSON3.Object{Base.CodeUnits{UInt8, String}, Vector{UInt64}} with 3 entries:
  :version        => "1.0.0"
  :primary_column => "geometry"
  :columns        => {

julia> json.columns.geometry.crs
JSON3.Object{Base.CodeUnits{UInt8, String}, SubArray{UInt64, 1, Vector{UInt64}, Tuple{UnitRange{Int64}}, true}} with 9 entries:
  Symbol("\$schema") => "https://proj.org/schemas/v0.6/projjson.schema.json"
  :type              => "GeographicCRS"
  :name              => "WGS 84 (CRS84)"
  :datum_ensemble    => {
  :coordinate_system => {
                    => 

Link to download the example.parquet file: https://github.com/opengeospatial/geoparquet/raw/v1.0.0/examples/example.parquet

@rafaqz
Copy link
Member

rafaqz commented Oct 3, 2024

Yes we are in the middle of fixing this globally using DataAPI metadata. See:

JuliaGeo/GeoInterface.jl#161

Maybe jump on to that issue if you have suggestions, or want to finish the PR with some tests and docs

@rafaqz
Copy link
Member

rafaqz commented Oct 3, 2024

You could also PR here to actually attach metadata to the DataFrame, followilling the standard in that PR

@eliascarv
Copy link
Contributor Author

Got it, I'll wait for the GeoInterface PR to be merged so I can make a PR adding the CRS to the metadata.

@eliascarv
Copy link
Contributor Author

It seems that the GeoInterface PR is already well underway, I will make the PR adding the CRS in the metadata

@asinghvi17
Copy link
Member

Yeah the standard is pretty set so feel free to add it. Loading GeoDataFrames will make this just work even now.

@visr
Copy link
Member

visr commented Oct 3, 2024

Regarding the other question by @eliascarv:

Perhaps returning a wrapper type that implements the Tables.jl interface would be a viable solution?

What is the reason to depend on DataFrames.jl for this package? Parquet2.Dataset already implements the Tables interface as well. Such an approach would also be more consistent with GeoJSON.jl and Shapefile.jl right?

@rafaqz
Copy link
Member

rafaqz commented Oct 3, 2024

That sounds like a better approach to me too.

We need the metadata either way but it should be attached to the Dataset

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.

4 participants