georust / proj

Rust bindings for the latest stable release of PROJ
https://docs.rs/proj
Apache License 2.0
137 stars 44 forks source link

Add `transform_bounds` to wrap `proj_trans_bounds` #155

Closed kylebarron closed 1 year ago

kylebarron commented 1 year ago

This adds a wrapper of transform_bounds, similar to pyproj's transform_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.

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:

michaelkirk commented 1 year ago

Should this handle radians/degrees like pyproj is doing?

I've not used pyproj, but upon quickly reviewing the documentation I haven't found what this refers to. Can you clarify?

michaelkirk commented 1 year ago

Sorry, I was looking at the wrong docs, instead of the ones you helpfully linked. 🤦

So it looks like the pyproj function takes a radians argument (default False) which when set to True will interpret your input as radians rather than degrees.

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.

michaelkirk commented 1 year ago

I'll wait a couple days months before merging in case someone else wants to take a look.

wow how time flies, sorry!

bors r+

bors[bot] commented 1 year ago

Build failed:

kylebarron commented 1 year ago

No worries! It looks like the ci failure is unrelated?

michaelkirk commented 1 year ago

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 https://github.com/georust/proj/pull/155#issuecomment-1581023392, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABU7YPBY5CP3WUL3UAQA23XKCKTDANCNFSM6AAAAAAWEKCM6M. You are receiving this because you commented.

michaelkirk commented 1 year ago

Ok, once https://github.com/georust/proj/pull/160 is merged we can retry.

michaelkirk commented 1 year ago

bors retry

bors[bot] commented 1 year ago

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. For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

kylebarron commented 1 year ago

Thanks!