search

rasterio / affine

Affine transformation matrices
https://affine.readthedocs.io/en/latest/index.html
BSD 3-Clause "New" or "Revised" License
162 stars 28 forks source link

Fix rotation matrix and __mul__ op #25

Closed sgillies closed 8 years ago

sgillies commented 8 years ago

The dot product of a rotation matrix and vector was right, but separately these were wrong.

coveralls commented 8 years ago

Coverage Status

Coverage remained the same at 98.052% when pulling 484ac5fe6408918036bdf200b47a2d2bd207623c on fix-matrix-mult-op into 0cd1cf269879c561697f37cc9ac00a5276828c98 on master.

sgillies commented 8 years ago

This PR has the __mul__ bug fix found by @ToddSmall and also gets the right-handed sense of rotation right. Here's a 90 degree ccw rotation of a unit vector example using numpy:

>>> from affine import Affine
>>> import numpy as np
>>> am = np.array(Affine.rotation(90.0)).reshape(3,3)
>>> am
array([[ 0., -1.,  0.],
       [ 1.,  0.,  0.],
       [ 0.,  0.,  1.]])
>>> point = np.array([1.0, 0.0, 1.0])
>>> np.dot(am, point)
array([  0.,  1.,   1.])

/cc @perrygeo @ToddSmall @mwtoews

mwtoews commented 8 years ago

Looks good. Here is one suggestion to consider for affine/__init__.py, line 229:

:param angle: Rotation angle in degrees, counter-clockwise about the pivot.

perrygeo commented 8 years ago

@sgillies Looks great - it's nice to have the rotation direction defined explicitly.

Could we also add @ToddSmall's test for associativity? https://github.com/sgillies/affine/pull/24/files#diff-3ebc029e55f312af9ae346c6433cd770R364

coveralls commented 8 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling 2dc816de6302a79b5032484bfa847864797d7cee on fix-matrix-mult-op into b4234eac1bb19a7c6e88db25aae299234598c0e1 on master.