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

WIP: Geo format type wrappers and generic methods #97

Merged
merged 9 commits into from
Apr 5, 2020

Conversation

rafaqz
Copy link
Collaborator

@rafaqz rafaqz commented Nov 13, 2019

No tests yet, but things like reproject do work for any format types, which is pretty powerful allready.

I'm not really familiar with the details of these formats or GDAL methods, so input on how to actually test these things and finish the PR will be needed.

My intent (and knowledge) is more around generalising the structure of these package and the ecosystem so we can do all kinds of transformations without needing all the method permutations everywhere.

For GeoData.jl this PR allows us to do GDALarray(filepath; reproject=EPSGcode(4326)) where you can pass in anything for reproject and GeoData doesn't need to know what it is, but your lat/lon coords will just work.

The type wrappers are coming from https://github.com/rafaqz/GeoFormatTypes.jl

@yeesian
Copy link
Owner

yeesian commented Nov 17, 2019

I'm not really familiar with the details of these formats or GDAL methods, so input on how to actually test these things and finish the PR will be needed.

To my knowledge, the spatial projection tests that we have are defined in the following files:

@rafaqz
Copy link
Collaborator Author

rafaqz commented Dec 27, 2019

Ok this is is tested, it just has to wait for the new version of GeoFormatTypes to go into the registry for tests to pass.

convert converts geometries or crs between most types in GeoFormatTypes. The mode::Geom and mode::CRS traits are used to handle multi-use formats like WellKnownText or GML. They will mostly be picked automatically in GeoFormatTypes.

reproject accepts any crs formats to generate the transform, and any GeoFormatTypes or AbstractGeometry geometry to be transformed. It also accepts tuple/vector coord formats (that could be cleaned up and standardised to fit GeoInterface etc at some point).

I'll rebase it against the other pull request once that's in, but otherwise this is done, so comments would be useful.

@rafaqz
Copy link
Collaborator Author

rafaqz commented Jan 5, 2020

This is rebased and passing now.

Copy link
Owner

@yeesian yeesian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this PR!

It seems to involve changes not just to the projections, but also to the Geometries right now, so I'll like to have a second pass at it after the requested changes.

src/ogr/geometry.jl Outdated Show resolved Hide resolved
src/spatialref.jl Outdated Show resolved Hide resolved
test/test_geometry.jl Outdated Show resolved Hide resolved
test/test_geometry.jl Show resolved Hide resolved
test/test_geometry.jl Outdated Show resolved Hide resolved
src/spatialref.jl Outdated Show resolved Hide resolved
src/spatialref.jl Outdated Show resolved Hide resolved
src/spatialref.jl Outdated Show resolved Hide resolved
src/spatialref.jl Outdated Show resolved Hide resolved
src/spatialref.jl Outdated Show resolved Hide resolved
@rafaqz
Copy link
Collaborator Author

rafaqz commented Apr 1, 2020

Ok I think this is pretty much finished, sorry for the delay! I've been using it for a few months in GeoData.jl and it's working well.

@yeesian yeesian merged commit b524e96 into yeesian:master Apr 5, 2020
@yeesian
Copy link
Owner

yeesian commented Apr 5, 2020

It looks great, thanks a lot! :)

@rafaqz
Copy link
Collaborator Author

rafaqz commented Apr 6, 2020

Thanks! I can relegister GeoData.jl whenever this is registered.

@rafaqz rafaqz deleted the formattypes branch April 6, 2020 02:49
@visr
Copy link
Collaborator

visr commented Apr 9, 2020

@rafaqz it will be registered in a few minutes: JuliaRegistries/General#12666

@yeesian yeesian mentioned this pull request Sep 19, 2020
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 this pull request may close these issues.

3 participants