Reintroduce GDAL extract projection and update extractprojection.py to QGIS 3

[havatv]

Fixes #20263 (when some other files are also fixed)

Description

Reintroduce GDAL extract projection and update extractprojection.py to QGIS 3

Checklist

Reviewing is a process done by project maintainers, mostly on a volunteer basis. We try to keep the overhead as small as possible and appreciate if you help us to do so by completing the following items. Feel free to ask in a comment if you have troubles with any of them.

• Commit messages are descriptive and explain the rationale for changes
• Commits which fix bugs include fixes #11111 in the commit message next to the description
• Commits which add new features are tagged with [FEATURE] in the commit message
• Commits which change the UI or existing user workflows are tagged with [needs-docs] in the commit message and contain sufficient information in the commit message to be documented
• I have read the QGIS Coding Standards and this PR complies with them
• This PR passes all existing unit tests (test results will be reported by travis-ci after opening this PR)
• New unit tests have been added for core changes
• I have run the scripts/prepare-commit.sh script before each commit

[luipir]

please add also relative test
see guide: http://www.opengis.ch/2016/02/04/increasing-the-stability-of-processing-algorithms/
or similar example in https://github.com/qgis/QGIS/blob/master/python/plugins/processing/tests/GdalAlgorithmsTest.py

tns for your contribution

[nyalldawson]

It might be nice to also add a file output here:

self.addOutput(QgsProcessingOutputFile(self.WORLD_FILE, self.tr('World file')))

and return the path to the newly created world file within the algorithm results. (Maybe also the optional prj file too?) This would make this algorithm more useful within a model context, as these filenames could be used with other later algorithms, such as copying to a different location, etc.

[havatv]

I have tried.

[havatv]

Not so easy to find examples of QgsProcessingOutputFile usage, but I have given it a try.

[havatv]

@luipir, I understand that tests would be useful, but could that be done in a second pull request? My focus was to fix a "bug". Now I am being asked for more... If this is merged, I will definitely do my best to make a test for the algorithm. Now that output is being generated, perhaps we could use the approach outlined in http://www.opengis.ch/2016/02/04/increasing-the-stability-of-processing-algorithms/

[havatv]

@luipir, I am considering these additions to the gdal_algorithm_tests.yaml file:

  - algorithm: gdal:extractprojection
    name: Extract Projection (extractprojection)
    params:
      INPUT:
        name: dem.tif
        type: raster
      PRJ_FILE_CREATE: True
    results:
      WORLD_FILE:
        name: expected/gdal/dem.wld
        type: file
      PRJ_FILE:
        name: expected/gdal/dem.prj
        type: file

  - algorithm: gdal:extractprojection
    name: Extract Projection (extractprojection)
    params:
      INPUT:
        name: dem.tif
        type: raster
      PRJ_FILE_CREATE: False
    results:
      WORLD_FILE:
        name: expected/gdal/dem.wld
        type: file
      PRJ_FILE: None

Would that work if I added the dem.wld and dem.prj to the python/plugins/processing/tests/testdata/expected/gdal/ directory?

[nyalldawson]

Thanks @havatv!

One thing I'm not sure about with this algorithm is what it does/is supposed to do with raster layers from multilayer sources, such as a geopackage.

My interpretation of the code is that the algorithm will likely crash on these inputs. Can you confirm? If so, i can point you to the correct way to handle these.

[havatv]

The code will only work for simple raster files (TIFF, JPEG, PNG, ...).
But I tested with a geopackage, and as expected there was an error:

with open(outFileName + '.wld', 'wt') as wld:
FileNotFoundError: [Errno 2] No such file or directory: 'GPKG:/home/user/geoptest.wld'
as expected (I guess creating an arbitrary file inside a geopackage is not supported).

This does not cause QGIS to crash. The error message is shown in the same way as when an exception is thrown in the algorithm code (the solution for handling non-GDAL sources).

As far as I know, it only makes sense to create world files for simple raster files, so getting an error when trying to do it for for instance a layer in a geopackage should be OK. The question is perhaps how helpful the error message should be.

Since we have added outputs, there is now a possible use case for just wanting to generate files for use "outside" the input layer. A possible future enhancement could be to allow the user to specify the location of the generated files. But I don't think that needs to be done now.

[havatv]

Is there any chance to "backport" this to 3.4, so that it will be available in the LTR? Should be low risk, and it is a quite useful function that may be missed by those that upgrade from QGIS 2.

[luipir]

@luipir, I am considering these additions to the gdal_algorithm_tests.yaml file:

Sorry for my late answer I was out... yes I suppose should work just adding the yaml and expected result in the correct directory. btw if you add a test you can simply run int and check if it works doing:
$> ctest -R

[havatv]

I have added the tests, as described above.

[luipir]

I have added the tests, as described above.

as you can see you should fix your test, before to push the PR, please test it locally
run it from you build directory with:
ctest -R ProcessingGdalAlgorithmsTest (add -V to have more verbose result)

you had the following error https://travis-ci.org/qgis/QGIS/jobs/451277706#L1594

[havatv]

Sorry, I did not understand that I also had to build before testing Python code. I will try to do a build.

[luipir]

Sorry, I did not understand that I also had to build before testing Python code. I will try to do a build.

no strictly necessary, you can always modify your PR and wait (a long) that travis fail to find the error... building locally allow you to speedup test development.
Waiting travis to approve your contribution, again thanks

[havatv]

I agree that it is better to run tests locally. But building QGIS can be a bit involved - it took me some time.

[havatv]

Anything else I should do before this is ready to be merged?

[luipir]

tnx @havatv

Thanks @havatv!

One thing I'm not sure about with this algorithm is what it does/is supposed to do with raster layers from multilayer sources, such as a geopackage.

My interpretation of the code is that the algorithm will likely crash on these inputs. Can you confirm? If so, i can point you to the correct way to handle these.

Sorry @nyalldawson I merged too early without your opinion... feel free to revert or propose a ask for a new PR if this PR does not satisfy your requests. BTW it does not crash as said by @havatv

[havatv]

Backporting to the LTR (3.4) should be done, as this fixes a regression in version 3. I guess that backport should be trivial.

[nyalldawson]

Yes please @havatv -- just open a PR with the backported fix to the release-3_4 branch and I'll merge