Implemented azimuthToBearing#2410
Implemented azimuthToBearing#2410basvdijk wants to merge 0 commit intoTurfjs:masterfrom basvdijk:master
Conversation
|
Implemented #2399 |
|
I think we can merge this :) |
rowanwins
left a comment
There was a problem hiding this comment.
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.
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.
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.
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.
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();
});
mfedderly
left a comment
There was a problem hiding this comment.
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 testat the sub modules where changes have occurred.npm run lintto ensure code style at the turf module level.Submitting a new TurfJS Module.
./scripts/generate-readmesto createREADME.md.package.jsonusing "Full Name <@github Username>".