-
Notifications
You must be signed in to change notification settings - Fork 10
add geointerface reprojection #79
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
Conversation
src/geointerface.jl
Outdated
The returned object will be constructed from `GeoInterface.WrapperGeometry` | ||
geometries, wrapping views of `Proj.Coord`. | ||
""" | ||
function reproject(geom, source_crs, target_crs; time=Inf) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add this as a method stub to GeoInterface so that other packages (ArchGDAL?) can specialize on this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does that make sense though? It wouldn't really give us anything more than we have.
The point of this PR is that it converts any geointerface object using Proj. There is no way to dispatch on that, so a stub would necessitate type piracy. I already wrote a ArchGDAL.reproject
method years ago. It will also handle any GeoInterface geometries when this is merged:
yeesian/ArchGDAL.jl#366
But you have to use ArchGDAL.reproject
for that to work. If we use a stub method in GeoInterface.jl then the packages can only handle types they own, so ArchGDAL.AbstractGeometry
for ArchGDAL.jl, and nothing for Proj.jl.
I'm trying to shift everything to package methods because the methods in GeoInterface (like area
etc) don't really make sense, there is no way to add useful methods to them without piracy or a backend system. Mostly I use Shapefile.jl and GeoJSON.jl directly, and need a way to work with those objects.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see what you're saying. Packages could implement functions which take arbitrary GeoInterface geometries and return their own internal structs then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Closed in favour of putting this in GeometryOps.jl |
This PR adds some basic GeoInterface.jl compatability for the
Coords
object, and uses that to add areproject
method that can reproject any geointerface geometry between source and target crs.reconstruct
could be moved to GeoInterface as it's quite generic - it rebuilds any nested geometry from a vector of points using wrapper types that match the original structure. Currently it's not type stable.One issue is that
GeoInterface.ncoord
doesn't work onCoords
- currently it returns 4 because its an array with x y z and timestamp, but there is no way we can detect if it is 2 or 3.We may have to settle for ncoord always being 3, and changing how the GeoInterface wrappers check point objects - we can let them have extra coords and use the wrapper to clamp them to hide the z. Currently the wrapper
Point
checks the number of coords in the wrapped objects and errors if it doesn't match.Another option is to add a type parameter to
Coord
to indicate if it is 2 or 3 dimensional.