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

Bugfix/point in polygon #1894

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

artoonie
Copy link

@artoonie artoonie commented Dec 8, 2022

Related issue
#1893

Description of changes
The current algorithm is based on https://stackoverflow.com/a/7123291
However, that algorithm fails when checking a point near the center of the polygon.

Just below that answer is https://stackoverflow.com/a/14998816

While I don't love that it still takes a list of lists, I have not changed that in order to maintain backwards-compatibility (see: #1873)

I'm not sure where tests would be added - I don't see any existing tests. Happy to add what's listed in the issue as a regression test if you can point me to where that'd be appropriate.

QA checklists

  • Add relevant code comments. Every API class and method should have <summary> description as well as description of parameters.
  • Add tests for new/changed/updated classes and methods!!!
  • Check out conventions in CONTRIBUTING.md.
  • Check out conventions in CODING-STYLE.md
  • Update the changelog
  • Update documentation.

Reviewers

Not sure who to tag.

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.

1 participant