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

Fix inverse datum shifting when coordinate is not exactly on a grid point #79

Merged
merged 1 commit into from
Apr 3, 2022

Conversation

rhuitl
Copy link
Contributor

@rhuitl rhuitl commented Apr 3, 2022

This is the same change as the one proposed in dwins/proj4j#3, which never got merged.

It aligns the nad_intr method in this repository with the original one from the C implementation: https://github.com/OSGeo/PROJ/blob/4.9/src/nad_intr.c#L54.

Further confirmation that this code is indeed missing: the Proj4JS implementation does this similarly: https://github.com/proj4js/proj4js/blob/master/lib/datum_transform.js#L187.

@pomadchin pomadchin self-requested a review April 3, 2022 16:59
Copy link
Member

@pomadchin pomadchin left a comment

Choose a reason for hiding this comment

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

Hey @rhuitl, gz with your first contribution and thanks for making this PR with a nice descirption. 🎉 I can merge it once the CI is green and all checks passed.

May I ask you to sign ECA please to make Eclipse license checker happy?

Eclipse grabs your name and email from the commit, so doublecheck that you commited under the correct email (that was used to sign ECA), right now the ECA is required for the Anonymous (git@****.de) email).

@rhuitl
Copy link
Contributor Author

rhuitl commented Apr 3, 2022

Hi @pomadchin, thank you for the prompt reply. I signed the ECA!

@pomadchin pomadchin merged commit 723b0f4 into locationtech:master Apr 3, 2022
@rhuitl rhuitl deleted the fix_nad_intr branch May 15, 2023 19:48
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.

None yet

2 participants