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

Processing gdalwarp issues #23608

Closed
qgib opened this issue Oct 9, 2016 · 25 comments
Closed

Processing gdalwarp issues #23608

qgib opened this issue Oct 9, 2016 · 25 comments
Labels
Milestone

Comments

@qgib
Copy link
Contributor

@qgib qgib commented Oct 9, 2016

Author Name: Giovanni Manghi (@gioman)
Original Redmine Issue: 15685
Affected QGIS version: master
Redmine category:processing/gdal
Assignee: Victor Olaya


Input: http://svn.simo-project.org/tools/trunk/txt2latlon/data/gt30w020n90.tif

On QGIS 2.16 with gdal 1.11 (QGIS Ubuntu 16.04 repos), try reprojecting to 3857 gives:

2.16 gdal 1.11
2016-10-09T21:14:26 2 Uncaught error while executing algorithm
Traceback (most recent call last):
File "/usr/share/qgis/python/plugins/processing/core/GeoAlgorithm.py", line 203, in execute
self.processAlgorithm(progress)
File "/usr/share/qgis/python/plugins/processing/algs/gdal/GdalAlgorithm.py", line 52, in processAlgorithm
commands = self.getConsoleCommands()
File "/usr/share/qgis/python/plugins/processing/algs/gdal/warp.py", line 179, in getConsoleCommands
if rastext and len(rastext_crs) > 0:
TypeError: object of type 'NoneType' has no len()

On master with gdal 1.11 (QGIS Ubuntu 16.04 repos) the "te" parameter has been added (automatically in fact), where with gdal 1.11 te_srs is not available. This seems to cause a lot of issues

GDAL command output:
ERROR 1: Attempt to create 0x0 dataset is illegal,sizes must be larger than zero.
Creating output file that is 0P x 0L.

in factI have not been able to reproject any raster.

On 2.16 with gdal 2.1 (QGIS Ubuntu 16.04 repos + Ubuntugis)

with the above input

gdalwarp -ot Float32 -t_srs EPSG:3857 -r near -of GTiff -te -20.0 40.0
20.0 90.0 -te_srs EPSG:4326 -co COMPRESS=DEFLATE -co PREDICTOR=1 -co
ZLEVEL=6 -wo OPTIMIZE_SIZE=TRUE gt30w020n90.tif OUTPUT.tif
GDAL command output:
ERROR 1: tolerance condition error
ERROR 1: -te_srs ignored since coordinate transformation failed.

if a smaller extent rather than -te -20.0 40.0
20.0 90.0 is choosen than works ok. This does not seems to be a problem with many other rasters, maybe the issue if the extent for this large wgs84 raster.

The issue here is not this one for me, the real issue is understand why "te" has been made mandatory (and bu consequence "te_srs" is automatically added/computed by Processing).

@qgib

This comment has been minimized.

Copy link
Contributor Author

@qgib qgib commented Oct 9, 2016

Author Name: Giovanni Manghi (@gioman)


Not sure if this affects also other gdal tools.

@qgib

This comment has been minimized.

Copy link
Contributor Author

@qgib qgib commented Oct 18, 2016

Author Name: Sandro Santilli (@strk)


I'm trying to reproduce, can you confirm I should be useing the "Warp (reproject)" processing tool ?
Opening the dialog of that tool, both with current 2.16 branch and master_2, I get an "Invalid value for parameter 'Destination SRS'", no matter setting it (by default it is unset). What are the exact steps you take to reproduce the subject issue ?


  • status_id was changed from Open to In Progress
  • assigned_to_id was changed from Victor Olaya to Sandro Santilli
@qgib

This comment has been minimized.

Copy link
Contributor Author

@qgib qgib commented Oct 18, 2016

Author Name: Sandro Santilli (@strk)


Giovanni: maybe it would be more useful to split this report in 3 different tickets ?
It's not easy to understand what you think should be done here. For example: are you saying you think -te should not be added with GDAL is < 2.0 ?

@qgib

This comment has been minimized.

Copy link
Contributor Author

@qgib qgib commented Oct 18, 2016

Author Name: Giovanni Manghi (@gioman)


Hi Sandro,

I'm sorry to me the description is clear and detailed

Sandro Santilli wrote:

I'm trying to reproduce, can you confirm I should be useing the "Warp (reproject)" processing tool ?

yes of course

Opening the dialog of that tool, both with current 2.16 branch and master_2, I get an "Invalid value for parameter 'Destination SRS'", no matter setting it
(by default it is unset). What are the exact steps you take to reproduce the subject issue ?

you must test with different qgis releseas compiled against gdal 1.* and 2.*

It is not complicated:

  • qgis 2.16, gdal 1.1, operation always fails with the abobe error

  • qgis master, gdal 1.1, the "te" parameter is used (in that gdal version there is not the "te_srs" one) and it causes the above error

  • using qgis 2.6/master with gdal 2.* it mostly work in any case, execpt for geographically large raster in geographic CRS:The "te" parameter is automagically added to the resulting command (and this anyway shuld not happen, should be optional) and the warping can fail. But in this specific cases you choose an extent for "te" that is slighly smaller than the input then it works. The latter is of course is a gdalwarp issue. The real issue in QGIS is that in this toll "te" must not be mandatory (and also te_srs when available).

I don't think we should split this ticket. It is all part of the very same tool/issue.

@qgib

This comment has been minimized.

Copy link
Contributor Author

@qgib qgib commented Oct 18, 2016

Author Name: Sandro Santilli (@strk)


And the issue is that "-te" is computed wrong ? Or "-te" should not be passed in at all ?

@qgib

This comment has been minimized.

Copy link
Contributor Author

@qgib qgib commented Oct 18, 2016

Author Name: Giovanni Manghi (@gioman)


Sandro Santilli wrote:

And the issue is that "-te" is computed wrong ? Or "-te" should not be passed in at all ?

it is not wrong per se, is the real extent of the input, but this does not play well in the gdalwarp command line command. But if you try with other rasters, smaller areas, different CRSs, it works. There is a problem in gdalwarp with that specific type of input. If only "te" was optional as it should be... at least the tool would become usable also in such cases (or of course the trick) is to choose a "te" extent smaller (even slightly smaller) that the input one.

@qgib

This comment has been minimized.

Copy link
Contributor Author

@qgib qgib commented Oct 19, 2016

Author Name: Sandro Santilli (@strk)


I'm trying to figure what sets that raster extent, beause at the time getConsoleCommands is reached (in algs/gdal/warp.py) the extent is already set to the layer extent.

@qgib

This comment has been minimized.

Copy link
Contributor Author

@qgib qgib commented Oct 19, 2016

Author Name: Sandro Santilli (@strk)


I found the culprit of non-optionality being gui/ExtentSelectionPanel, which takes an empty string as meaning "use min covering extent". If we want to make the extent optional we'll have to find another "special" value to signify "min covering extent".
How about "min" or "min-covering" or "auto" ?

@qgib

This comment has been minimized.

Copy link
Contributor Author

@qgib qgib commented Oct 19, 2016

Author Name: Giovanni Manghi (@gioman)


Sandro Santilli wrote:

I found the culprit of non-optionality being gui/ExtentSelectionPanel, which takes an empty string as meaning "use min covering extent". If we want to make the extent optional we'll have to find another "special" value to signify "min covering extent".
How about "min" or "min-covering" or "auto" ?

we already have non mandatory parameters in gdal/processing, we cannot just use the same as for them?

@qgib

This comment has been minimized.

Copy link
Contributor Author

@qgib qgib commented Oct 19, 2016

Author Name: Sandro Santilli (@strk)


The param is already non-mandatory, but the GUI currently intercept a blank value to signify "use min extent" rather than "do not specify an extent". Do you want to completely drop support for such "use min extent" behavior or do you want it to be done via a special value ?

@qgib

This comment has been minimized.

Copy link
Contributor Author

@qgib qgib commented Oct 19, 2016

Author Name: Sandro Santilli (@strk)


Giovanni can you test this change ?
#3638


  • done_ratio was changed from 0 to 20
  • pull_request_patch_supplied was changed from 0 to 1
@qgib

This comment has been minimized.

Copy link
Contributor Author

@qgib qgib commented Oct 19, 2016

Author Name: Giovanni Manghi (@gioman)


I tested your patches on master_2 (gdal 2.*).

If the "raster extent..." is left blank is all ok, no automatically added "te" pameter with the full extent of input.

On the other end if the extent is added

2016-10-19T17:38:21 2 Uncaught error while executing algorithm
Traceback (most recent call last):
File "C:/OSGeo4W/apps/qgis-dev/./python/plugins\processing\core\GeoAlgorithm.py", line 203, in execute
self.processAlgorithm(progress)
File "C:/OSGeo4W/apps/qgis-dev/./python/plugins\processing\algs\gdal\GdalAlgorithm.py", line 51, in processAlgorithm
commands = self.getConsoleCommands()
File "C:/OSGeo4W/apps/qgis-dev/./python/plugins\processing\algs\gdal\warp.py", line 203, in getConsoleCommands
return ['gdalwarp', GdalUtils.escapeAndJoin(arguments)]
File "C:/OSGeo4W/apps/qgis-dev/./python/plugins\processing\algs\gdal\GdalUtils.py", line 174, in escapeAndJoin
if s[0] != '-' and ' ' in s:
IndexError: string index out of range


  • done_ratio was changed from 20 to 0
@qgib

This comment has been minimized.

Copy link
Contributor Author

@qgib qgib commented Oct 19, 2016

Author Name: Sandro Santilli (@strk)


Fixed in changeset "dc2df62b88cdc01b0fc8839779da44fb87ccd213".


  • status_id was changed from In Progress to Closed
@qgib

This comment has been minimized.

Copy link
Contributor Author

@qgib qgib commented Oct 19, 2016

Author Name: Giovanni Manghi (@gioman)


The problems with gdal < 2.* has to be fixed and/or retested after this latest patches.


  • assigned_to_id removed Sandro Santilli
  • status_id was changed from Closed to Reopened
@qgib

This comment has been minimized.

Copy link
Contributor Author

@qgib qgib commented Oct 19, 2016

Author Name: Sandro Santilli (@strk)


Please file a new ticket if you see problems with GDAL-1.x.


  • assigned_to_id was configured as Sandro Santilli
  • status_id was changed from Reopened to Closed
@qgib

This comment has been minimized.

Copy link
Contributor Author

@qgib qgib commented Oct 19, 2016

Author Name: Giovanni Manghi (@gioman)


Sandro Santilli wrote:

Please file a new ticket if you see problems with GDAL-1.x.

why should I file a new ticket if this issue were reported here in the first place?


  • status_id was changed from Closed to Reopened
@qgib

This comment has been minimized.

Copy link
Contributor Author

@qgib qgib commented Oct 19, 2016

Author Name: Sandro Santilli (@strk)


Because this ticket reports too many bugs and is more useful to restrict the scope of tickets.
You can see from the ticket title how broad it is: "Processing gdalwrarp issues". "issues" ?
Please file a ticket for each issue.

@qgib

This comment has been minimized.

Copy link
Contributor Author

@qgib qgib commented Oct 19, 2016

Author Name: Giovanni Manghi (@gioman)


Sandro Santilli wrote:

Because this ticket reports too many bugs and is more useful to restrict the scope of tickets.
You can see from the ticket title how broad it is: "Processing gdalwrarp issues". "issues" ?
Please file a ticket for each issue.

they are not different issues, but different symptoms of the same buggy tool. There are no different "warp" tools depending on the different underlying libraries versions, it is just one. The user do not need to know anything about the underlying libraries. And for example if under Ubuntu I install QGIS usig the official qgis repo that does not make use of the Ubuntugis one what I get is QGIS compiled against gdal 1.1.*

@qgib

This comment has been minimized.

Copy link
Contributor Author

@qgib qgib commented Oct 19, 2016

Author Name: Sandro Santilli (@strk)


Fixed in changeset "06976a2e87737c755f5783243e09185e7c872e06".


  • status_id was changed from Reopened to Closed
@qgib

This comment has been minimized.

Copy link
Contributor Author

@qgib qgib commented Oct 20, 2016

Author Name: Victor Olaya (@volaya)


Sandro

I had to revert your change. It breaks many places where extents are used, and also it modifies the syntax of extent params (where empty or none means using min extent). I suggest we try to find a better solution in 3.0, but not in 2. I am planning a change for extents in 3, where extents can have an asigned CRS as well, so it makes sense to add this changes as well.

Sorry for the revert

@qgib

This comment has been minimized.

Copy link
Contributor Author

@qgib qgib commented Oct 20, 2016

Author Name: Sandro Santilli (@strk)


Ok, sorry for the light push (I was hoping the testsuite would have caught regressions)

For the record, master_2 revert is 8de56bf

Reopening and assigning to you.

Also, since master_2 won't be maintained and 2.14 is unaffected, I'm changing affected version to master and target version to 3.0, is that correct ?


  • version was changed from master_2 to master
  • status_id was changed from Closed to Reopened
  • assigned_to_id was changed from Sandro Santilli to Victor Olaya
  • fixed_version_id was changed from Version 2.18 to Version 3.0
@qgib

This comment has been minimized.

Copy link
Contributor Author

@qgib qgib commented Oct 20, 2016

Author Name: Sandro Santilli (@strk)


  • pull_request_patch_supplied was changed from 1 to 0
@qgib

This comment has been minimized.

Copy link
Contributor Author

@qgib qgib commented Oct 20, 2016

Author Name: Victor Olaya (@volaya)


yes, make sense to me

Thanks a lot!

@qgib

This comment has been minimized.

Copy link
Contributor Author

@qgib qgib commented Feb 9, 2017

Author Name: Alexander Bruy (@alexbruy)


As QGIS 3 will require GDAL 2.x I think we can close this, as with GDAL 2.x it works fine


  • status_id was changed from Reopened to Feedback
@qgib

This comment has been minimized.

Copy link
Contributor Author

@qgib qgib commented Feb 9, 2017

Author Name: Giovanni Manghi (@gioman)


  • resolution was changed from to wontfix
  • status_id was changed from Feedback to Closed
@qgib qgib added this to the Version 3.0 milestone May 25, 2019
@qgib qgib closed this May 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.