Implement rust-proj as an optional feature within geo #192
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
turboladen
reviewed
Mar 14, 2018
| // is signalled by the choice of enum used as input to the PJ_COORD union | ||
| // PJ_LP signals projection of geodetic coordinates, with output being PJ_XY | ||
| // and vice versa, or using PJ_XY for conversion operations | ||
| pub fn new(definition: &str) -> Option<Proj> { |
There was a problem hiding this comment.
Having never used PROJ directly, what's an example str that one would pass in here? WKT of the projection?
There was a problem hiding this comment.
Nevermind--I just realized that's a dumb question. 😬
|
Sweet! |
urschrei
force-pushed
the
reprojection
branch
4 times, most recently
from
e29c288 to
b5dac15
Compare
turboladen
reviewed
Mar 16, 2018
src/lib.rs
Outdated
| @@ -45,4 +51,6 @@ pub mod prelude { | |||
| pub use algorithm::from_postgis::FromPostgis; | |||
| #[cfg(feature = "postgis-integration")] | |||
| pub use algorithm::to_postgis::ToPostgis; | |||
| #[cfg(feature = "projection")] | |||
There was a problem hiding this comment.
Maybe I missed it, but I don't see projection as a defined feature--should this be feature = "proj"?
There was a problem hiding this comment.
It absolutely should. Fixed!
urschrei
force-pushed
the
reprojection
branch
2 times, most recently
from
9798583 to
4beefbc
Compare
Also make Project and Convert available as traits on any type that implements MapCoords
[ci skip]
Not totally sure how it's going to work yet
It depends on MapCoordsFallibleInplace, which is a bad idea
|
bors r+ |
bors bot
added a commit
that referenced
this pull request
Apr 8, 2018
192: Implement rust-proj as an optional feature within geo r=frewsxcv a=urschrei Continuing the discussion from #190 and #32. ## Introduction `rust-proj` is currently (as of `0.3.0`) using PROJ v4.9.x, and depends upon `Geo` for its `Point` type. However, we now have a `proj-sys` crate, with low-level bindings to PROJ v5.0.0. Unfortunately, `rust-proj's` dependency on `Geo` means that we can't directly use its functions within `Geo`, as this introduces a circular dependency (see #190). ### This PR: - Re-implements `rust-proj`'s functionality as an optional feature ("`proj`") within `Geo` using `proj-sys` - it has to be optional, as it requires PROJ v5.0.0 to be present - Makes two new traits –`Project` and `Convert` – available to any geometries implementing the `MapCoordsFallible` trait (see #191), allowing them to be (inverse) projected to and from Geodetic coordinates, and converted between projections. This functionality is a slightly more granular version (in keeping with the new PROJ API, which makes the same distinction) of what's possible using `pyproj` and `Shapely` geometries (see [here](http://shapely.readthedocs.io/en/stable/manual.html#other-transformations) for the official example): ```python from shapely.geometry import Point, LineString import pyproj # LineString in projected coordinates (OSGB36) ls = LineString([(548295.39, 182498.46), (548296.39, 182499.46)]) osgb36=pyproj.Proj("+init=EPSG:27700") eastings, northings = list(zip(*ls.coords)) # inverse-project to geodetic lon, lat coordinates (WGS 84) conv_ls = zip(*osgb36(eastings, northings, inverse=True)) # compare to command-line proj output assert conv_ls == [(0.13774816464635564, 51.52142699952049), (0.1377629902608001, 51.5214357231341)] ``` I propose that we leave `rust-proj` at 0.3.0, clearly marking it as deprecated, directing users to install `Geo` with the `proj` feature if they want v5.0.x features, and linking to `proj-sys` for implementers who don't wish to use `Geo`, for whatever reason. We can also (as per my and @turboladen's discussion in #32) highlight the availability of this feature more clearly, if need be. Co-authored-by: Stephan Hügel <urschrei@gmail.com>
Build succeeded |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Continuing the discussion from #190 and #32.
Introduction
rust-projis currently (as of0.3.0) using PROJ v4.9.x, and depends uponGeofor itsPointtype. However, we now have aproj-syscrate, with low-level bindings to PROJ v5.0.0. Unfortunately,rust-proj'sdependency onGeomeans that we can't directly use its functions withinGeo, as this introduces a circular dependency (see #190).This PR:
rust-proj's functionality as an optional feature ("proj") withinGeousingproj-sysProjectandConvert– available to any geometries implementing theMapCoordsFallibletrait (see Implement a MapCoordsFallible trait #191), allowing them to be (inverse) projected to and from Geodetic coordinates, and converted between projections. This functionality is a slightly more granular version (in keeping with the new PROJ API, which makes the same distinction) of what's possible usingpyprojandShapelygeometries (see here for the official example):I propose that we leave
rust-projat 0.3.0, clearly marking it as deprecated, directing users to installGeowith theprojfeature if they want v5.0.x features, and linking toproj-sysfor implementers who don't wish to useGeo, for whatever reason.We can also (as per my and @turboladen's discussion in #32) highlight the availability of this feature more clearly, if need be.