Add transform_bounds to wrap proj_trans_bounds
#155
Conversation
src/proj.rs
Outdated
| /// | ||
| /// ```rust | ||
| /// # use approx::assert_relative_eq; | ||
| /// extern crate proj; |
There was a problem hiding this comment.
I don't think we need this extern declaration since some years ago.
There was a problem hiding this comment.
I removed this extern crate as well as a couple others in docstrings
| bottom: F, | ||
| right: F, | ||
| top: F, | ||
| densify_pts: i32, |
There was a problem hiding this comment.
A doc about densify pts would be nice.
There was a problem hiding this comment.
I added a sentence taken from the pyproj docs
| right: F, | ||
| top: F, | ||
| densify_pts: i32, | ||
| ) -> Result<[F; 4], ProjError> |
There was a problem hiding this comment.
Though it's pretty much just like pyproj does, it seems a little strange to me that the input and output formats are different. (left, bottom, right, top) -> [F; 4]
We could do like we did with transform and accept some kind of Bounds trait.
Or we could output a bespoke Bounds { left, bottom, right, top }struct (or maybe reuse/consolidate with the existing proj::Area).
One thing I considered, if we did go the Bounds trait route, it'd be natural to want to do:
impl proj::Bounds for geo_types::Rect {
...
}
... but I think that might give us some incorrect results when we span the prime meridian - with how you have it now, it's no problem if left > right. I'm not actually sure what happens if you try it though.
Or we can just leave it. Apparently it's good enough for pyproj.
There was a problem hiding this comment.
it seems a little strange to me that the input and output formats are different
That's fair. Open to change it to return a tuple of F or use a struct. I'm not too familiar with existing crate conventions.
when we span the prime meridian
It looks like proj allows the max value to be less than the min when it spans the antimeridian.
I've not used pyproj, but upon quickly reviewing the documentation I haven't found what this refers to. Can you clarify? |
|
Sorry, I was looking at the wrong docs, instead of the ones you helpfully linked. 🤦 So it looks like the pyproj function takes a I prefer how you have it: assuming degrees only for now. If we eventually wanted to support radians in the proj crate, it seems like it might have ramifications in Proj/ProjBuilder (e.g.). Regardless, I'd say it's fine (desirable even) to leave it as degrees only for now. |
wow how time flies, sorry! bors r+ |
155: Add `transform_bounds` to wrap `proj_trans_bounds` r=michaelkirk a=kylebarron This adds a wrapper of `transform_bounds`, similar to pyproj's `transform_bounds` ([docs](https://pyproj4.github.io/pyproj/stable/api/proj.html#pyproj.Proj.transform_bounds), [code](https://github.com/pyproj4/pyproj/blob/79a8602a2b0b50bf72c072b5a31317570bbdf550/pyproj/_transformer.pyx#L935-L1018)). This also adds a quick doctest, which is derived from one of the above doctests, but which I ran in pyproj to get the output data to compare against. ```py from pyproj import CRS, Transformer start = CRS("EPSG:2230") end = CRS("EPSG:26946") transformer = Transformer.from_crs(start, end, always_xy=True) transformer.transform_bounds(4760096.421921, 3744293.729449, 4760196.421921, 3744393.729449) # (1450880.2910605017, # 1141263.0111604768, # 1450910.7711214642, # 1141293.4912214405) ``` Questions: - Should this handle radians/degrees like pyproj is doing? Co-authored-by: Kyle Barron <kylebarron2@gmail.com>
|
Build failed: |
|
No worries! It looks like the ci failure is unrelated? |
|
Indeed. I can take a look at CI tomorrow.
… On Jun 7, 2023, at 08:09, Kyle Barron ***@***.***> wrote:
No worries! It looks like the ci failure is unrelated?
—
Reply to this email directly, view it on GitHub <#155 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABU7YPBY5CP3WUL3UAQA23XKCKTDANCNFSM6AAAAAAWEKCM6M>.
You are receiving this because you commented.
|
|
Ok, once #160 is merged we can retry. |
|
bors retry |
|
Build succeeded! The publicly hosted instance of bors-ng is deprecated and will go away soon. If you want to self-host your own instance, instructions are here. If you want to switch to GitHub's built-in merge queue, visit their help page.
|
|
Thanks! |
This adds a wrapper of
transform_bounds, similar to pyproj'stransform_bounds(docs, code).This also adds a quick doctest, which is derived from one of the above doctests, but which I ran in pyproj to get the output data to compare against.
Questions: