NaturalGIS / naturalgis_ntv2_transformations

A plugin for the QGIS Processing toolbox to allow users do Datum transformations with NTv2 grids
Other
14 stars 8 forks source link

Added new grid for Bavaria; Fixed "grid not found error" on Mac OS #46

Open vmarquar opened 4 years ago

vmarquar commented 4 years ago

Hey Guys, please see my attached pull request to integrate into your plugin.

Short Description:

  1. Fix for Issue: nadgrids not found under Mac OS X
    • 1.1. Problem: The default plugin directory under Mac OS X contains a whitespace character > "Application Support", e.g. /Users/<Username>/Library/Application Support/QGIS/QGIS3/profiles/default/python/plugins/ntv2_transformations/grids
    • The best way would be to escape the whitespace character with \ or wrap it into "...". But that's not possible inside the proj-string definition (see Proj#1152, Proj#218).
    • 1.2. Current implemented workaround:
    • Inside transformations.py I added some code to create a hidden symbolic link which is placed inside the user's home directory and references the plugin directory. Not the best solution but I think it's acceptable.
  2. Added new transformation for the grid BY_KANU
    • create two new files for the two transformations VectorDE_BY... and RasterDE_BY...
    • created a download error message which instructs the user to download the big grid file (1.03 GB download size) and move it to the grid directory.

If anything is unclear don't hesitate to contact me.

Kind regards Valentin Fixes #45

gioman commented 4 years ago

@vmarquar many thanks for your PR! have you tested the changes on all major OSes (Linux, Windows and macOS)?

vmarquar commented 4 years ago

I added your suggestions. So far I have only tested the plugin under MacOS, but I can double check the plugin under Windows in the upcoming days. I will let you know if there are any Windows specific issues.

gioman commented 4 years ago

I added your suggestions.

Thanks!

So far I have only tested the plugin under MacOS, but I can double check the plugin under Windows in the upcoming days. I will let you know if there are any Windows specific issues.

We would appreciate it. We can test on Linux. Cheers!

gioman commented 4 years ago

@alexbruy thanks for the review!

vmarquar commented 4 years ago

So I checked the Plugin under Windows 10 (QGIS 3.6.2) and my adaptions seem to work fine. There is only one problem where QGIS is not guessing the EPSG Code correctly (it guesses EPSG:3397 PD/83 instead of EPSG:5678/EPSG:31468). Should I try the -a tag in the gdal command to assign a Code explicitly?

However, I found another bigger bug which is affecting all urlretrieve statements. When running behind a Proxy Server (or you don't have an Internet connection at all) the Plugin chrashes with a timeoutError. We should at least rise an exception with a correct error message, which informs the user to download the files manually. Another Option would be to implement a download solution which is getting the proxy configuration from the QGIS Settings (like e.g. using the QgsNetworkAccessManager class) or just bundle the small .gsb files directly with the plugin.

gioman commented 4 years ago

So I checked the Plugin under Windows 10 (QGIS 3.6.2) and my adaptions seem to work fine.

@vmarquar Nice, many thanks for your effort. That QGIS version is anyway too old especially because it uses ancient GDAL and Proj versions compared to the latest QGIS ltr. Any test at the moment must be done against QGIS installations using GDAL 3 and Proj 6/7.

There is only one problem where QGIS is not guessing the EPSG Code correctly (it guesses EPSG:3397 PD/83 instead of EPSG:5678/EPSG:31468). Should I try the -a tag in the gdal command to assign a Code explicitly?

What GDAL and Proj versions are used by the installations you have tested with? In the QGIS bug tracker a few similar issues have been reported and in most cases they were fixed upstream in GDAL or Proj. It is possible that the fixes have not yet landed in QGIS packages. You should test this hypotheses by doing a transformation from the command line using ogr2ogr and then loading the results in QGIS. Of course we can also test if -a_srs helps anyway.

However, I found another bigger bug which is affecting all urlretrieve statements. When running behind a Proxy Server (or you don't have an Internet connection at all) the Plugin chrashes with a timeoutError. We should at least rise an exception with a correct error message, which informs the user to download the files manually. Another Option would be to implement a download solution which is getting the proxy configuration from the QGIS Settings (like e.g. using the QgsNetworkAccessManager class)

I see, but this scenarios were never really in the scope of the plugin. Of course supporting this cases would be a nice improvement, but unfortunately right now we can't put any effort in it.

just bundle the small .gsb files directly with the plugin.

There is a "no binaries in plugins code" policy.

vmarquar commented 4 years ago

@gioman I've conducted the test with following QGIS installation parameters:

You are right. My installation is indeed very old and I will try to update QGIS and will let you know if the problem still exists after the upgrade. Btw, a short test using gdalwarp on the command line didn't work as the -a_srs parameter doesn't get accepted by gdalwarp.

gioman commented 4 years ago
  • GDAL/OGR | 2.4.1

  • PROJ | 5.2.0 | Rel. 5.2.0, September 15th, 2018

@vmarquar hi, yes as said there is not point in testing any QGIS installation that does not use GDAL 3 and Proj 6/7. CRSs handling has changed a lot in this GDAL/Proj versions so we must test on them to see if any change is needed the way plugin uses ogr2ogr or gdalwarp.

Btw, a short test using gdalwarp on the command line didn't work as the -a_srs parameter doesn't get accepted by gdalwarp.

no, but gdal_translate has it. So if needed we may think use translate instead of warp (or we can pipe the result of warp in a translate command).

gioman commented 2 years ago

added small improvements.

@vmarquar thanks for the follow up. I will review it ASAP.