misleading documentation in `angle_vectors_signed()`

[obucklin]

The description of angle_vectors_signed(u, v, normal, deg=False, tol=None) is confusing and I've been using it wrong for the past year. I expected it to act like a projection plane with the normal as the normal parameter. what it does instead is to just take the smallest angle between the two vectors and then make it negative if the cross product doesn't match the normal.

what happens

>>> angle_vectors_signed([0.0, 1.0, 0.0], [1.0, 1.0, 0.0], [0.0, 0.0, 1.0]) 
-0.7853981633974484
>>> angle_vectors_signed([1.0, 1.0, 0.0], [0.0, 1.0, 0.0], [0.0, 0.0, 1.0])  
0.7853981633974484
>>> angle_vectors_signed([0.0, 1.0, 0.0], [1.0, 1.0, 5.0], [0.0, 0.0, 1.0]) 
-1.37713802635057
>>>

Expected behavior I expected the angle to be calculated as if rotated(and viewed) from the normal vector, e.g.

>>> angle_vectors_signed([0.0, 1.0, 0.0], [1.0, 1.0, 0.0], [0.0, 0.0, 1.0]) 
-0.7853981633974484
>>> angle_vectors_signed([0.0, 1.0, 0.0], [1.0, 1.0, 5.0], [0.0, 0.0, 1.0]) 
-0.7853981633974484
>>>

I would also expect the angle to change based on the order in which the two vector parameters are given:

>>> angle_vectors_signed([0.0, 1.0, 0.0], [1.0, 1.0, 0.0], [0.0, 0.0, 1.0]) 
-0.7853981633974484
>>> angle_vectors_signed([1.0, 1.0, 0.0], [0.0, 1.0, 0.0], [0.0, 0.0, 1.0])  
5.49778714378213
>>>

I expect we can't "fix" this in terms of changing behavior without breaking everything, but maybe change the description.

I would suggest: Returns the smallest angle between 2 vectors, with the sign of the angle based on the direction of the normal vector according to the right hand rule of rotation.

I would submit this as a bug fix, but my compas folder is not connected to the repo for some reason... This is maybe a bit of a feature request, too, to get that sweet, sweet expected behavior.

[tomvanmele]

yes, you make a good point :) would you mind submitting a PR with the suggested change?

[chenkasirer]

@obucklin nice. I've been scratching my head at this one for a while, too. This is essentially what you expect it to do, right?:

# angle between `ref_side.xaxis` and `plane.normal` projected on plane with normal `ref_side.zaxis`
angle_vector = Vector.cross(ref_side.zaxis, plane.normal)
angle = angle_vectors_signed(ref_side.xaxis, angle_vector, ref_side.zaxis, deg=True)

I can help with setting you up for submitting a PR.

[obucklin]

hey I'm glad to make these changes. I'll add a function and change the doc on the existing function to avoid breaking stuff

[Licini]

Closing since it's done : )