-
Notifications
You must be signed in to change notification settings - Fork 944
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
Implemented azimuthToBearing #2410
Conversation
Implemented #2399 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can merge this :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Requires a tidyup of the function so that the tests pass
packages/turf-helpers/index.ts
Outdated
* @returns {number} bearing between -180 and +180 degrees | ||
*/ | ||
export function azimuthToBearing(angle: number): number { | ||
return angle < 0 ? angle + 360 : angle; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pretty sure this isn't working as expected @basvdijk now that I've run CI and seen the failing tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this works 😃:
function azimuthToBearing(angle) {
angle = angle % 360;
if (angle > 0) return angle > 180 ? angle - 360 : angle;
return angle < -180 ? angle + 360 : angle;
}
test('azimuthToBearing', t => {
t.equal(azimuthToBearing(0), 0);
t.equal(azimuthToBearing(360), 0);
t.equal(azimuthToBearing(180), 180);
t.equal(azimuthToBearing(40), 40);
t.equal(azimuthToBearing(40 + 360), 40);
t.equal(azimuthToBearing(-35), -35);
t.equal(azimuthToBearing(-35 - 360), -35);
t.equal(azimuthToBearing(255), -105);
t.equal(azimuthToBearing(255 + 360), -105);
t.equal(azimuthToBearing(-200), 160);
t.equal(azimuthToBearing(-200 - 360), 160);
t.end();
});
packages/turf-helpers/index.ts
Outdated
* and returns an angle between -180 and +180 degrees (positive clockwise), 0 being the north line | ||
* | ||
* @name azimuthToBearing | ||
* @param {number} angle between 0 and 360 degrees |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think angle
should not be restricted to 0-360, but any number, you can easily normalize it with mod 360
packages/turf-helpers/index.ts
Outdated
* @returns {number} bearing between -180 and +180 degrees | ||
*/ | ||
export function azimuthToBearing(angle: number): number { | ||
return angle < 0 ? angle + 360 : angle; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this works 😃:
function azimuthToBearing(angle) {
angle = angle % 360;
if (angle > 0) return angle > 180 ? angle - 360 : angle;
return angle < -180 ? angle + 360 : angle;
}
test('azimuthToBearing', t => {
t.equal(azimuthToBearing(0), 0);
t.equal(azimuthToBearing(360), 0);
t.equal(azimuthToBearing(180), 180);
t.equal(azimuthToBearing(40), 40);
t.equal(azimuthToBearing(40 + 360), 40);
t.equal(azimuthToBearing(-35), -35);
t.equal(azimuthToBearing(-35 - 360), -35);
t.equal(azimuthToBearing(255), -105);
t.equal(azimuthToBearing(255 + 360), -105);
t.equal(azimuthToBearing(-200), 160);
t.equal(azimuthToBearing(-200 - 360), 160);
t.end();
});
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm open to merging this but can you update this branch and incorporate the suggestions from stebogit? Thanks!
@mfedderly just added the fixes according to comment of @stebogit |
It looks like you picked up a merge conflict along the way can you knock that out too? |
@mfedderly somehow I can't get it to work. I keep getting issues, merge conflicts and I am currently unable to make it work. I guess it has something to do with being 100 commits behind after a year. |
It might have been that you just needed to pull from the turf repo instead of your fork? If you're just opening a new PR can you @ me so I see it? Thanks. |
@mfedderly Created #2674 to replace this PR |
Please fill in this template.
npm test
at the sub modules where changes have occurred.npm run lint
to ensure code style at the turf module level.Submitting a new TurfJS Module.
./scripts/generate-readmes
to createREADME.md
.package.json
using "Full Name <@github Username>".