-
Notifications
You must be signed in to change notification settings - Fork 7
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
Add great-circle meridian crossing option #159
Conversation
This commit adds an option to compute the intersection of a segment and the meridian using spherical geometry, which provides nicer splits for large crossing the meridian diagonally. Related to gadomski#153
@gadomski I have local changes which propagate a CLI option |
I think it should be the default behavior, it looks much nicer. So the CLI option would be better if is disabled great circle behavior, IMO. |
This propagates the decision of whether to use planar or spherical geometry to compute meridian crossing latitudes. The default is set to False to keep this commit self-contained, because changing the default has a significant impact on the test results. The change of default will be done in a later commit, which will update the test suite at the same time. See gadomski#159 (comment)
This provides a way to opt out of the default spherical geometry used for computing meridian crossings.
This prepares the addition of test outputs expected from spherical geometry.
This is required to introduce an optional argument with a default value (subdirectory), which cannot be expressed with Callable type hinting only. This has the nice benefit of not removing the need for typing extensions, since Protocols are available since Python 3.8, which is lower than the oldest non-EOL Python version. See https://docs.python.org/3/library/typing.html#annotating-callable-objects
The output of ogr2ogr is consistent with the behaviour of great-circle meridian crossing computation introduced by this PR.
The differences have been checked visually and are convincing, except the force-north-pole test for which a double-check would be needed.
Some tests were kept unparametrized over great_circle to make sure that the default behaviour is also tested. See gadomski#159 (comment) BREAKING CHANGE: Meridian crossings are now computed using spherical geometry by default.
All code-related changes should be in. How do we go about updating the docs ? The drawings in |
Nope, that's ok! No need to do the docs, I'll take that on if needed. Really appreciate it! |
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 is really excellent 🥳! Thanks for the sidecar typing fixes, etc.
Related issues and pull requests
Description
This PR introduces support for splitting polygons crossing the meridian using spherical geometry rather than 2D, which provides nicer splits for large objects meridian diagonally.
For example, the polygon referenced in #153, which was originally fixed like this:
would now look like that:
Checklist