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

Add NTv2 multi subgrid support #74

Merged

Conversation

Rennzie
Copy link
Contributor

@Rennzie Rennzie commented Nov 10, 2023

Adds support for multiple NTv2 subgrids. The implementation is adapted from the FGRID subroutine described in the archived spec (pg 42).

I've done my best to be fully compliant to the FGRID rules of NTv2. There may be some edge cases missing though.

@busstoptaktik
Copy link
Owner

@Rennzie: In #75, I corrected the interpolation bug, and added a tiny NTv2 file (built using the ntv2_file tool from Esri's NTv2 file routines). This should be sufficient to test your new code, without adding a large dependency, with unclear licensing (the new file is less than 2 kB).

I spent quite some time trying to dig up an actual license for the Saskatchewan file, and while I suspect it is covered by the Government of Saskatchewan Standard Unrestricted Use Data Licence Version 2.0), I cannot know for sure, and I would really prefer not to pollute the licensing space of RG with any non-interoperable special-purpose bessermachen licenses. It's CC-BY, CC0 and really not anything else.

So I think my hand-crafted solution is the better alternative, and it was easier and cheaper than hiring a lawyer to untangle things 😊

To be absolutely sure the Saskatchewan file does not accidentally get merged through multiple-commit-history, I would prefer to see this PR restarted as a new branch, branching off from main, and not including the Saskatchewan file at any moment in its commit history. I.e. not just rebasing (because then commit history and hence historically committed material will creep into the repo) but hand held direct diffing of the relevant files from branch to branch. Or, since you may very well speak git more fluently than I do: Whichever way you see as right - just be sure that 1.8 MB of Saskatchewan never enters the repo, even as part of the commit history: It will just live on forever under the carpets!

That said: There is a minor conflict in subgrid.rs to resolve, due to my updates, and your tests will have to be against the 5458_with_subgrid.gsb file. But otherwise I think this is shaping up nicely!

@Rennzie Rennzie force-pushed the Add-NTv2-multi-subgrid-support branch from 2681734 to 8d4745e Compare November 11, 2023 09:42
@Rennzie
Copy link
Contributor Author

Rennzie commented Nov 11, 2023

@busstoptaktik thats absolutely no problem. I'll remove all references, commits etc to that .gsb file. I think I can do it in this branch as it was only a single commit that references it. I'll update the description too.

Thanks for adding a multi level .gsb. I'll be able to write better tests now too. And thanks also for fixing the precision of the grid interpolation. That will be extremely useful. I'll be testing a number of different .gsb files for work soon so this is timely 🎉

@Rennzie Rennzie force-pushed the Add-NTv2-multi-subgrid-support branch from 8d4745e to 8f8b8b5 Compare November 11, 2023 10:05
@Rennzie Rennzie force-pushed the Add-NTv2-multi-subgrid-support branch from 3cd469c to 068dca5 Compare November 11, 2023 14:50
@Rennzie
Copy link
Contributor Author

Rennzie commented Nov 11, 2023

@busstoptaktik this is ready for another look. With your hand rolled subgrid .gsb I've managed to make the find_grid method more sophisticated than it was. Really keen to see what you think.

@busstoptaktik
Copy link
Owner

@Rennzie:

Really keen to see what you think.

I think this looks really good! - I'll merge immediately, and then update #76 for compatibility

@busstoptaktik busstoptaktik merged commit 2418e1f into busstoptaktik:main Nov 14, 2023
1 check 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.

2 participants