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.

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

Use great circles to calculate crossing latitude #153

Closed
teobouvard opened this issue Nov 5, 2024 · 3 comments · Fixed by #159
Closed

Use great circles to calculate crossing latitude #153

teobouvard opened this issue Nov 5, 2024 · 3 comments · Fixed by #159
Labels
enhancement New feature or request

Comments

@teobouvard
Copy link
Contributor

Hello,

I'm using the following polygon as input

{
    "type": "Polygon",
    "coordinates": [
        [
            [
                179.45064648,
                -65.47124346
            ],
            [
                178.30851744,
                -65.99077528
            ],
            [
                179.74817611,
                -66.51221218
            ],
            [
                -179.1162027,
                -65.98206677
            ]
        ]
    ]
}
Loading

When running

antimeridian fix poly.json

I'm getting the following result

{
    "type": "MultiPolygon",
    "coordinates": [
        [
            [
                [
                    180.0,
                    -65.726774
                ],
                [
                    179.45064648,
                    -65.47124346
                ],
                [
                    178.30851744,
                    -65.99077528
                ],
                [
                    179.74817611000003,
                    -66.51221218
                ],
                [
                    180.0,
                    -66.2469064
                ],
                [
                    180.0,
                    -65.726774
                ]
            ]
        ],
        [
            [
                [
                    -180.0,
                    -66.2469064
                ],
                [
                    -179.1162027,
                    -65.98206677
                ],
                [
                    -180.0,
                    -65.726774
                ],
                [
                    -180.0,
                    -66.2469064
                ]
            ]
        ]
    ]
}
Loading

which does not seem correct ? I was expecting the lines between the four vertices to be ~straight, but they're not. At least I did not expect it to be a discontinuity at the antimeridian, but I might make wrong assumptions. Could this be due to crossing_latitude not performing great circle intercepts with the 180° meridian ?
@gadomski
Copy link
Owner

gadomski commented Nov 5, 2024

Could this be due to crossing_latitude not performing great circle intercepts with the 180° meridian ?

That's correct, we don't:

antimeridian/src/antimeridian/_implementation.py

Lines 501 to 512 in 0f4ccb1

if end[0] > 0:
return round(
start[1]
+ (180.0 - start[0]) * latitude_delta / (end[0] + 360.0 - start[0]),
7,
)
else:
return round(
start[1]
+ (start[0] + 180.0) * latitude_delta / (start[0] + 360.0 - end[0]),
7,
)

I'll mark this as an enhancement 👍🏼

@gadomski gadomski added the enhancement New feature or request label Nov 5, 2024
@gadomski gadomski changed the title Incorrect fix of rectangle polygon Use great circles to calculate crossing latitude Nov 5, 2024
@teobouvard
Copy link
Contributor Author

Thanks, good to know! Would you be open to a PR for this ? I see the only dependencies are shapely and numpy, neither of which provide great-circle computations directly. Would you prefer to rely on a package providing this utility or have this implemented in the antimeridian package ?

@gadomski
Copy link
Owner

gadomski commented Nov 5, 2024

Would you be open to a PR for this ?

Yes! That would be excellent.

Would you prefer to rely on a package providing this utility or have this implemented in the antimeridian package ?

I try to stay dependency-light if possible, and IIRC it's not that complicated of an equation, so if we can implement it directly w/o too much hassle that would be my preference.

teobouvard added a commit to teobouvard/antimeridian that referenced this issue Dec 11, 2024
@teobouvard
feat: add great-circle meridian crossing option
5097fbd 
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
@teobouvard teobouvard mentioned this issue Dec 11, 2024
Merged
3 tasks
gadomski pushed a commit that referenced this issue Dec 18, 2024
@teobouvard
feat: add great-circle meridian crossing option (#159)
a054124 
* feat: add great-circle meridian crossing option

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 #153

* feat: add great_circle argument to functions

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 #159 (comment)

* feat: parametrize tests over great_circle values

* feat: add --no-great-circle option to CLI

This provides a way to opt out of the default spherical geometry used
for computing meridian crossings.

* feat: move output test data to subdirectory

This prepares the addition of test outputs expected from spherical
geometry.

* feat: refactor Reader type alias as protocol

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

* test: add great-circle polygon fixing test

The output of ogr2ogr is consistent with the behaviour of great-circle
meridian crossing computation introduced by this PR.

* docs: add entry to changelog

* test: add great circle test suite

The differences have been checked visually and are convincing, except
the force-north-pole test for which a double-check would be needed.

* feat!: change default value of great_circle arg

Some tests were kept unparametrized over great_circle to make sure that
the default behaviour is also tested.

See #159 (comment)

BREAKING CHANGE: Meridian crossings are now computed using spherical
geometry by default.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add great-circle meridian crossing option teobouvard/antimeridian
2 participants
@gadomski @teobouvard