rasterio / affine

Affine transformation matrices
BSD 3-Clause "New" or "Revised" License
156 stars 28 forks source link

Rotate by radians ? #61

Closed stuaxo closed 3 years ago

stuaxo commented 3 years ago

In Affine, rotations are always in degrees, but it can sometimes be useful to expose radians.

If you are considering changing the API, would you consider exposing an API that works in radians ?

mwtoews commented 3 years ago

Both rotate and shear functions could support an additional use_radians=False argument. See (e.g.) shapely.affinity.rotate where angles in radians are preserved, if required. This enhancement may require a separate cos_sin_rad helper function that "snaps" to a few key angles (maybe inexact "multiples" of pi).

stuaxo commented 3 years ago

Both rotate and shear functions could support an additional use_radians=False argument. See (e.g.) shapely.affinity.rotate where angles in radians are preserved, if required.

An alternative is to have rotation_radians and make rotation a wrapper of it.

This enhancement may require a separate cos_sin_rad helper function that "snaps" to a few key angles (maybe inexact "multiples" of pi).

Should there be a way to set the snap distance / disable snapping ?

mwtoews commented 3 years ago

An alternative is to have rotation_radians and make rotation a wrapper of it.

Whatever is simplest. I'd figure adding an argument would be an easy approach. Keep in mind that the vast majority of users will be using degrees, so I'd prefer to keep this interface simple to see/learn/debug.

Should there be a way to set the snap distance / disable snapping ?

The "snapping" that I mention is to avoid values that are near 0, e.g. these values should all be snapped to zero:

>>> import numpy as np
>>> np.sin(np.pi)
1.2246467991473532e-16
>>> np.sin(2*np.pi)
-2.4492935982947064e-16
>>> np.cos(np.pi/2)
6.123233995736766e-17
>>> np.cos(3*np.pi/2)
-1.8369701987210297e-16
stuaxo commented 3 years ago

An alternative is to have rotation_radians and make rotation a wrapper of it.

Whatever is simplest. I'd figure adding an argument would be an easy approach. Keep in mind that the vast majority of users will be using degrees, so I'd prefer to keep this interface simple to see/learn/debug.

That makes sense.

Should there be a way to set the snap distance / disable snapping ?

The "snapping" that I mention is to avoid values that are near 0...

Oh, I see - I thought this was about angles close to multiples of 90 degrees.

mwtoews commented 3 years ago

Oh, I see - I thought this was about angles close to multiples of 90 degrees.

These are! 90=pi/2, 180=pi, 270=3pi/2, 360=2pi

stuaxo commented 3 years ago

My fault for not staring at the numbers enough :)

I can imagine an image editor might even want 45 degrees, but this may be YAGNI as I'm not making one right at this moment.

mwtoews commented 3 years ago

Hmm, looking around the unit circle there are potentially other angle snap opportunities:

>>> import numpy as np
>>> np.sqrt(2)/2  # exact 45
0.70710678118654757
>>> np.sin(np.pi/4)
0.70710678118654746
>>> np.cos(np.pi/4)
0.70710678118654757
>>> np.sin(np.pi/6)
0.49999999999999994

however, where does it end? I'm not sure how much benefit there could be, so I'm inclined to not do too much snapping. The biggest gain is identifying zero from sin/cos at the 90 degree intervals, otherwise very small coefficient values can creep in affine matrices.

stuaxo commented 3 years ago

The plotdevice project has an interesting API for rotations, where there is a basis parameter, that can be RADIANS, DEGREES, and PERCENT.

In PERCENT the value is 0.0 to 1.0.

sgillies commented 3 years ago

While affine would be more consistent with Python's math module if it took radians instead of degrees, I'm :-1: on accepting both. For applications that are based on radians there is very little extra cost in writing rotation(math.degrees(angle)). @mwtoews I know that I agree to use_radians in https://github.com/Toblerity/Shapely/blob/master/shapely/affinity.py#L132 but I don't want to reproduce that pattern.