GeoTools
  1. GeoTools
  2. GEOT-4063

NTv2 grid shift transformation (EPSG:9615)

    Details

    • Type: New Feature New Feature
    • Status: Resolved Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 8.0
    • Component/s: referencing
    • Labels:
      None
    • Testcase included:
      yes

      Description

      Notes:

        Issue Links

          Activity

          Hide
          Andrea Aime added a comment -
          Oscar, cannot look at the patch right now, but one notice to get started, the grid file is probably a bit too large. Is there any way to get only a subset of it (without getting crazy doing so, that is)
          Show
          Andrea Aime added a comment - Oscar, cannot look at the patch right now, but one notice to get started, the grid file is probably a bit too large. Is there any way to get only a subset of it (without getting crazy doing so, that is)
          Hide
          Oscar Fonts added a comment -
          We can use BALR2009.gsb (41 kb compressed). Not in EPSG, but will work for unit tests. I'll have to change them, though.
          Show
          Oscar Fonts added a comment - We can use BALR2009.gsb (41 kb compressed). Not in EPSG, but will work for unit tests. I'll have to change them, though.
          Hide
          Andrea Aime added a comment -
          If it's not too trouble it would be better indeed
          Show
          Andrea Aime added a comment - If it's not too trouble it would be better indeed
          Hide
          Oscar Fonts added a comment -
          Show
          Oscar Fonts added a comment - Changed tests to use BALR2009.gsb. See https://github.com/oscarfonts/geotools/commit/0a7367b789b80453ceba95aa2ab3884a2afee32f
          Hide
          Andrea Aime added a comment -
          Hi Oscar,
          I took your patch for a walk and transformed it a bit. The new patch is geot-4063-aa-full.patch, meant to be applied using "git apply" since it also contains some binary files. Unfortunately I did not pay attention and the patch is cumulative, that is, not on top of your work, but on top of a vanilla geotools checkout. If you want I can also try to provide a diff on top of your first patch, the second one I forgot to commit into git before moving on with my refactors :-(

          The core of the code did not change, but made it so that we plugin a grid locator instead of the full grid factory, and that the whole work is applicable not only to NTv2, but also to NADCON, and added tests for the latter as well.
          The extension point is now simply:

          {code}
          public interface GridShiftLocator extends Factory {

          /**
          * Locate the specified resource.
          *
          * @param grid the grid name/location
          * @return the fully resolved URL of the grid or null, if the resource cannot be located.
          */
          URL locateGrid(String grid);
          }
          {code}

          There is a default implementation doing classpath lookups like before, but the interface is now used by both ntv2 and nadcon caching factories.
          I also added the smallest nadcon grid shift files I could find and created tests using the official USA transformation tool for reference.
          The idea is that GeoServer will have its own grid shift locator that allows the grid shift files to be put in the data directory.

          Now it's your turn to review and tell me if what I did is ok, and propose improvements :-)

          Ah, there is one problem with the test you provided, I believe this bit in the license is incompatible with the LGPL:

          {code}
          In the event that you wish to alter the grid file which
          appears at this ZIP file , for a purpose other than
          personal, in-house or non-commercial use, you can apply to
          the Instituto Geogr�fico Nacional at the following address
          for formal permission.
          {code}

          Afaik you cannot limit the purpose of the usage of anything that is in Geotools.
          I'm going to start a discussion on the devel list to see if that is ok for test code, otherwise I believe GeoTools can use the French grid shift file (ntf_r93.gsb), which afaik has no such redistribution limitations.
          Show
          Andrea Aime added a comment - Hi Oscar, I took your patch for a walk and transformed it a bit. The new patch is geot-4063-aa-full.patch, meant to be applied using "git apply" since it also contains some binary files. Unfortunately I did not pay attention and the patch is cumulative, that is, not on top of your work, but on top of a vanilla geotools checkout. If you want I can also try to provide a diff on top of your first patch, the second one I forgot to commit into git before moving on with my refactors :-( The core of the code did not change, but made it so that we plugin a grid locator instead of the full grid factory, and that the whole work is applicable not only to NTv2, but also to NADCON, and added tests for the latter as well. The extension point is now simply: {code} public interface GridShiftLocator extends Factory { /** * Locate the specified resource. * * @param grid the grid name/location * @return the fully resolved URL of the grid or null, if the resource cannot be located. */ URL locateGrid(String grid); } {code} There is a default implementation doing classpath lookups like before, but the interface is now used by both ntv2 and nadcon caching factories. I also added the smallest nadcon grid shift files I could find and created tests using the official USA transformation tool for reference. The idea is that GeoServer will have its own grid shift locator that allows the grid shift files to be put in the data directory. Now it's your turn to review and tell me if what I did is ok, and propose improvements :-) Ah, there is one problem with the test you provided, I believe this bit in the license is incompatible with the LGPL: {code} In the event that you wish to alter the grid file which appears at this ZIP file , for a purpose other than personal, in-house or non-commercial use, you can apply to the Instituto Geogr�fico Nacional at the following address for formal permission. {code} Afaik you cannot limit the purpose of the usage of anything that is in Geotools. I'm going to start a discussion on the devel list to see if that is ok for test code, otherwise I believe GeoTools can use the French grid shift file (ntf_r93.gsb), which afaik has no such redistribution limitations.
          Hide
          Oscar Fonts added a comment -
          Hi Andrea,

          Ok, I ended up making GridShiftFactories, when the GridLocator idea was much simpler than that. :)

          The only thing I miss is the possibility to control the order in which different grid locators are run. Simply making GridLocators extend AbstractFactory would enable priority handling.

          PS. Yes, official spanish grid files are non derivative...
          Show
          Oscar Fonts added a comment - Hi Andrea, Ok, I ended up making GridShiftFactories, when the GridLocator idea was much simpler than that. :) The only thing I miss is the possibility to control the order in which different grid locators are run. Simply making GridLocators extend AbstractFactory would enable priority handling. PS. Yes, official spanish grid files are non derivative...
          Hide
          Andrea Aime added a comment -
          Ah ok, extending AbstractFactory is fine by me
          Show
          Andrea Aime added a comment - Ah ok, extending AbstractFactory is fine by me
          Hide
          Oscar Fonts added a comment -
          The incremental patch w/AbstractFactory added to GridLocators.
          Show
          Oscar Fonts added a comment - The incremental patch w/AbstractFactory added to GridLocators.
          Hide
          Andrea Aime added a comment -
          Hi Oscar, I've just committed the overall patch on trunk at r38364.
          Before backporting it to 2.7.x I want to release GeoTools 2.7.5/GeoServer 2.1.4, we're very close and this change is large, not entirely comfortable sticking it directly a few days before a release in a stable series that is towards the end of its stabilitization.
          By backporting it to the stable series after the release we allow 1-2 months for people to start using it on nightlies and eventually report issues. Of course we'll blog about the changes so that everybody know they are there and go look for nightly builds
          Show
          Andrea Aime added a comment - Hi Oscar, I've just committed the overall patch on trunk at r38364. Before backporting it to 2.7.x I want to release GeoTools 2.7.5/GeoServer 2.1.4, we're very close and this change is large, not entirely comfortable sticking it directly a few days before a release in a stable series that is towards the end of its stabilitization. By backporting it to the stable series after the release we allow 1-2 months for people to start using it on nightlies and eventually report issues. Of course we'll blog about the changes so that everybody know they are there and go look for nightly builds
          Hide
          Oscar Fonts added a comment -
          Yes, better wait for the next release and let us & others test it thoroughly.
          Show
          Oscar Fonts added a comment - Yes, better wait for the next release and let us & others test it thoroughly.
          Hide
          Ben Caradoc-Davies added a comment -
          Patch causes build failures. See GEOT-4080.
          Show
          Ben Caradoc-Davies added a comment - Patch causes build failures. See GEOT-4080 .
          Hide
          Ben Caradoc-Davies added a comment -
          BALR2009_copyright_and_disclaimer.txt also has some non-ASCII non-UTF-8 characters that are going to get corrupted. Can we convert these to UTF-8? Do we have a best-practice or even an svn property to handle these?
          Show
          Ben Caradoc-Davies added a comment - BALR2009_copyright_and_disclaimer.txt also has some non-ASCII non-UTF-8 characters that are going to get corrupted. Can we convert these to UTF-8? Do we have a best-practice or even an svn property to handle these?
          Hide
          Oscar Fonts added a comment -
          The gold rule should be: **use UTF8**. :) Re-encoded BALR2009_copyright_and_disclaimer.txt
          Show
          Oscar Fonts added a comment - The gold rule should be: **use UTF8**. :) Re-encoded BALR2009_copyright_and_disclaimer.txt
          Hide
          Ben Caradoc-Davies added a comment -
          Note: the build failures reported in GEOT-4080 are now fixed.
          Show
          Ben Caradoc-Davies added a comment - Note: the build failures reported in GEOT-4080 are now fixed.
          Hide
          Oscar Fonts added a comment -
          Bug in NTv2Transform hashCode and equals methods. See GEOT-4093.
          Show
          Oscar Fonts added a comment - Bug in NTv2Transform hashCode and equals methods. See GEOT-4093 .
          Hide
          Jody Garnett added a comment -
          So is this one still open? Release candidate is going out ... but activity on this issue has died off.
          Show
          Jody Garnett added a comment - So is this one still open? Release candidate is going out ... but activity on this issue has died off.
          Hide
          Oscar Fonts added a comment -
          It's working in 8.0-RC1, but the code is not backported to 2.7.x.
          We agreed to backport it after 2.7.5 release.
          I think this is the reason why Andrea keeps this open.
          Show
          Oscar Fonts added a comment - It's working in 8.0-RC1, but the code is not backported to 2.7.x. We agreed to backport it after 2.7.5 release. I think this is the reason why Andrea keeps this open.
          Hide
          Andrea Aime added a comment -
          Yep, that was the original plan many months ago. The changes have been pretty extensive in the meantime, wondering, with the gt 8.x / gs 2.2.x getting into stable state do we still need to do a backport?
          Show
          Andrea Aime added a comment - Yep, that was the original plan many months ago. The changes have been pretty extensive in the meantime, wondering, with the gt 8.x / gs 2.2.x getting into stable state do we still need to do a backport?
          Hide
          Oscar Fonts added a comment -
          Whatever you decide will be OK for me.
          Show
          Oscar Fonts added a comment - Whatever you decide will be OK for me.

            People

            • Assignee:
              Andrea Aime
              Reporter:
              Oscar Fonts
            • Votes:
              1 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: