search

JuliaGeo / GeoJSON.jl

Utilities for working with GeoJSON data in Julia
https://juliageo.org/GeoJSON.jl/stable/
MIT License
68 stars 10 forks source link

Handle GeoFormatTypes `GeoJSON` objects #32

Closed rafaqz closed 2 years ago

rafaqz commented 2 years ago

It would be good if an object we already know is GeoFormatTypes.GeoJSON would pass through GeoJSON.write. It will already be a Dict or String. The string is assumed to be correctly formatted JSON so would not need anything done. A Dict would go to JSON3.write.

We could even wrap the output as a GeoJSON string, but maybe that's going to far for now... It would be like JSString in WebIO.jl that wraps javascipt strings.

The use case for this is to allow users to pass in various objects that need different degrees of processing to be formatted as GeoJSON, and they all work.

yeesian commented 2 years ago

I'm open to improving the interoperability of the packages/objects across JuliaGeo.

My slight hesitation with GeoFormatTypes is that the type system/hierarchy in it still looks experimental rather than a fully fleshed out design. E.g. I can't say I know how to interpret https://github.com/JuliaGeo/GeoFormatTypes.jl/blob/6ea1afad2eaeb78f6f0a7d468edd1dbb7d324705/src/GeoFormatTypes.jl#L201-L206 the way I'd know how interpret a string that meets the spec in https://geojson.org/. But I could be misunderstanding its current status/maturity, and the types in GeoFormatTypes might possibly just need documentation with examples and wider visibility.

rafaqz commented 2 years ago

Totally forgot about this. @yeesian the GeoJSON wrapper is explicitly saying that the internal string follows the GeoJSON standard, that's basically all that it does. But I will make this clearer in the docs, I'm putting some time into that currently.

yeesian commented 2 years ago

The documentation and state of GeoFormatTypes.jl has improved quite a bit since https://github.com/JuliaGeo/GeoJSON.jl/issues/32#issuecomment-953463705 (really great work!) and I agree it will be nice for this package.

visr commented 2 years ago

Fixed by #42.