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

Abstract types for compatibility with LibGEOS #15

Closed
maximerischard opened this issue Apr 26, 2017 · 3 comments
Closed

Abstract types for compatibility with LibGEOS #15

maximerischard opened this issue Apr 26, 2017 · 3 comments

Comments

@maximerischard
Copy link

If I have, say, a GeoJSON.LineString

ls_json = GeoJSON.LineString([[1.0,2.0],[3.0,4.0]])

and would like to convert it to a LibGEOS.LineString

LibGEOS.LineString(ls_json)

I get a MethodError

MethodError: Cannot `convert` an object of type GeoJSON.LineString to an object of type LibGEOS.LineString
This may have arisen from a call to the constructor LibGEOS.LineString(...),
since type constructors fall back to convert methods.

 in LibGEOS.LineString(::GeoJSON.LineString) at ./sysimg.jl:53

LibGEOS does have a method for converting any GeoInterface.AbstractLineString to a LibGEOS.LineString:

methods(LibGEOS.LineString)
# 4 methods for generic function "(::Type)":
…
LibGEOS.LineString{T<:GeoInterface.AbstractLineString}(obj::T) at ~/.julia/v0.5/LibGEOS/src/geos_types.jl:36
(::Type{T}){T}(arg) at sysimg.jl:53

but GeoJSON.LineString is not a GeoInterface.AbstractLineString

LibGEOS.LineString <: GeoInterface.AbstractLineString
true
GeoJSON.LineString <: GeoInterface.AbstractLineString
false

and so I end up having to do the conversion myself

LibGEOS.LineString(ls_json.coordinates)

which admittedly isn't the end of the world, but it would be convenient and elegant if GeoJSON.LineString were a subtype of GeoInterface.AbstractLineString.

@yeesian
Copy link
Member

yeesian commented Apr 26, 2017

Yeah, you're right (:

I have a local branch that introduces GeoInterface as a dependency for this package, but had been holding off on it because I was worried users might prefer this package to be "standalone".

Having an issue like this is a good signal (to me) that it might not be a bad way to proceed, so thanks!

@maximerischard
Copy link
Author

maximerischard commented Apr 26, 2017 via email

@yeesian
Copy link
Member

yeesian commented Apr 27, 2017

See #16.

@yeesian yeesian closed this as completed Apr 27, 2017
rafaqz added a commit that referenced this issue Jun 30, 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