Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

turf.angle does not return signed angle as claimed #2703

Closed
4 tasks done
wchargin opened this issue Aug 23, 2024 · 2 comments · Fixed by #2714
Closed
4 tasks done

turf.angle does not return signed angle as claimed #2703

wchargin opened this issue Aug 23, 2024 · 2 comments · Fixed by #2714
Assignees

Comments

@wchargin
Copy link

wchargin commented Aug 23, 2024

I'm trying to get the behavior of PostGIS ST_Angle in client-side JavaScript, so I turned to @turf/angle. Compare docs:

ST_Angle: Computes the clockwise angle […] enclosed by the points P1-P2-P3

@turf/angle: Finds the angle formed by two adjacent segments defined by 3 points. The result will be the (positive clockwise) angle with origin on the startPointmidPoint segment

These both look like they do what I want: in particular, they both specify that they return the clockwise angle. So I can determine whether a point B lies to the right or the left of my line OA by checking whether the result of angle(A, O, B) is in [0°, 180°] or [180°, 360°]:

$ psql -tc "SELECT degrees(ST_Angle('POINT(0 1)', 'POINT(0 0)', 'POINT(1 0)'))"
      90

$ psql -tc "SELECT degrees(ST_Angle('POINT(0 1)', 'POINT(0 0)', 'POINT(-1 0)'))"
     270

But Turf gives the same answer for both queries:

> turf = require("@turf/turf")  // v7.0.0
> turf.angle([0, 1], [0, 0], [1, 0])  // okay
90
> turf.angle([0, 1], [0, 0], [-1, 0])  // wrong!
90

I tested on version 7.0.0. I also see this behavior represented in test cases:

t.equal(round(angle([5, 5], [5, 6], [3, 4])), 45, "45 degrees");
t.equal(round(angle([3, 4], [5, 6], [5, 5])), 45, "45 degrees -- inverse");

Isn't this in error? The positive-clockwise angle from A=(0, 1) to B=(−1, 0) with origin O=(0, 0) is not 90°; it is 270° or −90°. The only points 90° clockwise of A are (x, 0) for x > 0.

I see #1192, which discusses this issue and raises a similar question (see the second diagram in particular, with the blue solid and dashed angles). But that PR "just added a few test cases to confirm and highlight the current behavior" and did not actually either change the default behavior or add an absolute option.

What is the best way to get the documented functionality within Turf? I can compute bearing(O, B) - bearing(O, A) and normalize to [−180°, 180°], but it seems like that's roughly what @turf/angle does before it calls Math.abs and throws away the useful sign information that it promises to return, which seems strange to me so I want to check if there's something that I'm missing.


  • The version of Turf you are using, and any other relevant versions: v7.0.0
  • GeoJSON data: not necessary; see repros above
  • Snippet of source code: see above
  • Verify this issue hasn't already been reported: searched for clockwise angle and read all issues
@smallsaucepan
Copy link
Member

Thanks for putting this together @wchargin. Seems pretty clear angle isn't doing what it says on the tin.

Looking at the implementation there are a few things we seem to be doing oddly. We get the bearing from A to 0 (180) rather than O to A (0), etc. Then compute AO - BO, rather than OB - OA. Maybe those two reversals cancel each other out - mostly.

Reversing those and removing the abs() call gives 90 and 270 as you'd expect for your example. It breaks other tests though, so will need to review those.

What is the best way to get the documented functionality within Turf? I can compute bearing(O, B) - bearing(O, A) ...

Don't think you're missing anything. The above might be the best stopgap for the time being.

@smallsaucepan smallsaucepan self-assigned this Aug 30, 2024
@wchargin
Copy link
Author

Thank you for your reply! I appreciate the second pair of eyes. I'll keep my workaround for the time being and happily switch back to @turf/angle if updated.

smallsaucepan added a commit to smallsaucepan/turf that referenced this issue Sep 15, 2024
… docs. Reviewed the implementations of azimuthToBearing, radiansToDegrees, and degreesToRadians to be easier to follow. Small discrepancies in the angle() geojson test fixtures caused by the bearing be taken from the wrong end of each line needed to be fixed. These tests included additional illustrative info meaning they were overly fragile though, so retired those in favour of vanilla unit tests which should be sufficient for a simple function like angle.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants