Skip to content

Changed turf-destination and turf-ellipse to pass through elevation value if present #2852

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
May 1, 2025
Merged

Changed turf-destination and turf-ellipse to pass through elevation value if present #2852

merged 5 commits into from
May 1, 2025

Conversation

prozvora
Copy link
Contributor

@prozvora prozvora commented Mar 8, 2025 •
edited
Loading

Summary of Changes

Fix elevation in destination.
Add input and output test cases with elevation to destination and ellipse.

Note

This is a non-breaking change.

Resolves

Test Output

turf-full-test-output

Destination

# turf-destination
ok 1 point-way-far-away-miles
ok 2 point-way-far-away-km
ok 3 point-90
ok 4 point-180
ok 5 point-0
ok 6 point-0-with-elevation

1..6
# tests 6
# pass  6

# ok

Ellipse

# turf-ellipse
anti-meridian-degrees.json
ok 1 anti-meridian-degrees
anti-meridian.json
ok 2 anti-meridian
northern-latitudes-degrees.json
ok 3 northern-latitudes-degrees
northern-latitudes.json
ok 4 northern-latitudes
rotation-degrees.json
ok 5 rotation-degrees
rotation.json
ok 6 rotation
simple-degrees.json
ok 7 simple-degrees
simple-with-elevation.json
ok 8 simple-with-elevation
simple.json
ok 9 simple
# turf-ellipse -- circle consistency
ok 10 both areas are equal
# turf-ellipse -- rotation consistency
ok 11 both areas are equal
# turf-ellipse -- with coordinates
ok 12 should be truthy
# turf-ellipse -- validate geojson
ok 13 (unnamed assert)

1..13
# tests 13
# pass  13

# ok

Please provide the following when creating a PR:

  • Meaningful title, including the name of the package being modified.
  • Summary of the changes.
  • Heads up if this is a breaking change.
  • Any issues this resolves.
  • Inclusion of your details in the contributors field of package.json - you've earned it! 👏
  • Confirmation you've read the steps for preparing a pull request.

prozvora added 2 commits March 8, 2025 15:58
@prozvora
Fix coordinate elevation component failing to pass though.
8d6e5b8 
Fix dimension handling in tesselate.
Test for elevation in tesselate.
Fix elevation in destination.
Add input and output test cases with elevation to destination and ellipse.
@prozvora
Add self to contributors.
a331a9e
prozvora
prozvora commented Mar 8, 2025
View reviewed changes
packages/turf-tesselate/test.ts Outdated
Comment on lines 3521 to 3535
t.equal(
simpleTrianglesWithElevation.type,
"FeatureCollection",
"Polygon returns a FeatureCollection"
);
t.equal(
simpleTrianglesWithElevation.features[0].geometry.type,
"Polygon",
"contains at least 1 triangle"
);
t.equal(
simpleTrianglesWithElevation.features[0].geometry.coordinates[0].length,
4,
"triangle is valid"
);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure if these tests are necessary to repeat. I copied them from the first polygon. I figured they would catch any malformation issues from adding the elevation to the coordinate.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed tessellate changes, should be included in a different PR as they are more complex.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #2855

@prozvora prozvora changed the title turf-destination, turf-ellipse, turf-tesselate: Fix coordinate elevation component failing to pass though. turf-destination, turf-ellipse: Fix coordinate elevation component failing to pass though. Mar 16, 2025
@prozvora
Copy link
Contributor Author

@smallsaucepan can I get a review?

@smallsaucepan
Copy link
Member

Apologies @prozvora. I'm a bit swamped at the moment. Will take a look for you as soon as I can 👍

@prozvora
Copy link
Contributor Author

Apologies @prozvora. I'm a bit swamped at the moment. Will take a look for you as soon as I can 👍

@smallsaucepan Bumping, can I get a review?

Also, could this be patched on v6.5.0?

@prozvora
Copy link
Contributor Author

@twelch any chance I could get a review on this?

smallsaucepan
smallsaucepan requested changes Apr 29, 2025
View reviewed changes
Copy link
Member

@smallsaucepan smallsaucepan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry this took so long to get to @prozvora. Have added one request to check at runtime whether any elevation elements passed are numbers.

smallsaucepan
smallsaucepan approved these changes May 1, 2025
View reviewed changes
@smallsaucepan smallsaucepan changed the title turf-destination, turf-ellipse: Fix coordinate elevation component failing to pass though. Changed turf-destination and turf-ellipse to pass through elevation value if present May 1, 2025
@smallsaucepan smallsaucepan merged commit 3b8e7fe into Turfjs:master May 1, 2025
3 checks passed
@smallsaucepan
Copy link
Member

Thanks for this PR @prozvora 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@smallsaucepan smallsaucepan smallsaucepan approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@prozvora @smallsaucepan