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

Added new point-to-polygon-distance package #2735

Merged

Conversation

pachacamac
Copy link
Contributor

@pachacamac pachacamac commented Oct 25, 2024

Please fill in this template. Use a meaningful title for the pull request. Include the name of the package modified.

  • Is this a bug fix, new functionality, or a breaking change?
    • new functionality
  • Have read and followed the steps for preparing a pull request.

Submitting a new TurfJS Module.

  • Overview description of proposed module.
  • Include JSDocs with a basic example.
  • Execute ./scripts/generate-readmes to create README.md.
  • Add yourself to contributors in package.json using "Full Name <@github Username>".

Resolves #1743

@smallsaucepan
Copy link
Member

smallsaucepan commented Oct 26, 2024

Looking good thanks @pachacamac. Glad to see it works with polygon holes as well.

Before it's ready to merge we might need to flesh it out in a couple of areas:

  1. Allowing options in line with sister functions like pointToLineDistance. For example, being able to pass Units (meters, miles, yards).
  2. Accepting the widest possible "complexity" of meaningful arguments. So, for the point a user can pass a simple number array (Position), or a bare geometry (Point), or a full on feature with properties (Feature<Point>). So the arguments would look like this instead:
    point: Feature<Point> | Point | Position
    polygon: Feature<Polygon | MultiPolygon> | Polygon | MultiPolygon

Sorry if that's more involved than you expected. Happy to co-author though if your time is limited.

@pachacamac
Copy link
Contributor Author

Thanks for pointing out the options. Missed that I can just forward this to the pointToLineDistance function. I also added a test that roughly reflects the initial issue question. For the sake of not complicating my test fixture setup I won't print out which point is the closest but pointB is the closest ;) which can be checked by comparing the expected distances of the three fixture points in the long-lines-poly.geojson.
I also added the expected unit to the test fixtures to validate that the unit conversion is working as expected.

@smallsaucepan smallsaucepan changed the title turf-point-to-polygon-distance package #1743 Added new point-to-polygon-distance package Oct 27, 2024
@pachacamac
Copy link
Contributor Author

@smallsaucepan anything else needed from my side?

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.

Thanks for this @pachacamac. Looks good.

One final task - could you please add this to the packages/turf/ package as well? That's the aggregator package that pulls in all the other turf-* packages for easy importing.

Pretty sure all you need to do is add point-to-polygon-distance as a package dependency, and re-export the main function from index.ts

@pachacamac
Copy link
Contributor Author

@smallsaucepan done 👍

@smallsaucepan
Copy link
Member

Thanks @pachacamac. Let's give it a spin through CI.

…d. Re-adding new package as a dependency in my env and letting lock file regenerate to previous format.
@smallsaucepan
Copy link
Member

Little bump in the road related to the pnpm-lock file. Pushed a quick fix and CI is green, so this is ready to merge 🎉

Thanks for your work on this @pachacamac. This was our most 👍-ed outstanding issue.

@smallsaucepan smallsaucepan merged commit 5d1e0ec into Turfjs:master Nov 14, 2024
3 checks passed
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 this pull request may close these issues.

Distance to Polygon / MultiPolygon from Point
2 participants