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

Widen the scope of GeoJSON.write #34

Closed
rafaqz opened this issue Apr 19, 2022 · 6 comments
Closed

Widen the scope of GeoJSON.write #34

rafaqz opened this issue Apr 19, 2022 · 6 comments

Comments

@rafaqz
Copy link
Member

rafaqz commented Apr 19, 2022

It seems that write does not work with all AbstractGeometry. I'm not sure if that is intentional. But currently e.g. ArchGDAL.IGeometry does not work.

julia> using GADM, GeoJSON

julia> GeoJSON.write(GADM.get("MUS").geom[1])
ERROR: MethodError: no method matching write(::ArchGDAL.IGeometry{ArchGDAL.wkbMultiPolygon})
Closest candidates are:
  write(::AbstractFeatureCollection) at ~/.julia/packages/GeoJSON/JVqvX/src/GeoJSON.jl:40
  write(::AbstractGeometryCollection) at ~/.julia/packages/GeoJSON/JVqvX/src/GeoJSON.jl:40
  write(::AbstractFeature) at ~/.julia/packages/GeoJSON/JVqvX/src/GeoJSON.jl:40
  ...
Stacktrace:
 [1] top-level scope
   @ REPL[33]:1

In the docs we say that any AbstractGeometry will work:

help?> GeoJSON.write
  write(obj)

  Create a GeoJSON string from an object that implements the GeoInterface, either AbstractGeometry,
  AbstractFeature or AbstractFeatureCollection.

The current workaround is to wrap in a Feature.

@rafaqz
Copy link
Member Author

rafaqz commented Apr 19, 2022

Is there a reason we have a limited list of types dispatched on here, instead of just using AbstractGeometry ?
https://github.com/JuliaGeo/GeoJSON.jl/blob/master/src/GeoJSON.jl#L53-L57

@visr
Copy link
Member

visr commented Apr 19, 2022

I think that can indeed just be AbstractGeometry. As far as I can see all of the current GeoInterface subtypes of AbstractGeometry are supported by the GeoJSON format. ArchGDAL.IGeometry should work through geo2dict.

@rafaqz
Copy link
Member Author

rafaqz commented Apr 19, 2022

Ok, I'll change that

@visr
Copy link
Member

visr commented Jul 3, 2022

The example above, GeoJSON.write(GADM.get("MUS").geom[1]), now works, since #42. We still need to test this however. Currently only objects from this package are tested. We could test with GADM/ArchGDAL, or perhaps just something small like https://github.com/evetion/WellKnownGeometry.jl.

@rafaqz
Copy link
Member Author

rafaqz commented Jul 11, 2022

Yes it would be nice to test this. Leaflet.jl already tests it with GADM.jl/ArchGDAL.jl, so maybe WellKnownGeometry.jl.

@rafaqz
Copy link
Member Author

rafaqz commented Aug 27, 2022

This works now

@rafaqz rafaqz closed this as completed Aug 27, 2022
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

No branches or pull requests

2 participants