Bug report #21452

r.series broken in Processing due to wrong "range=0,0" pre-definition

Added by Markus Neteler about 5 years ago. Updated about 5 years ago.

Status:Open
Priority:High
Assignee:-
Category:Processing/GRASS
Affected QGIS version:3.7(master) Regression?:Yes
Operating System: Easy fix?:No
Pull Request or Patch supplied:No Resolution:
Crashes QGIS or corrupts data:No Copied to github as #:29269

Description

In QGIS 3.4, 3.6 r.series is broken in Processing due to wrong "range=0,0" pre-definition. It should be empty but is pre-populated with 0,0.

That leads to an empty result (basically a NO DATA raster map):

[...]
g.proj -c proj4="+proj=lcc +lat_1=36.16666666666666 +lat_2=34.33333333333334 +lat_0=33.75 +lon_0=-79 +x_0=609601.22 +y_0=0 +ellps=GRS80 +towgs84=0,0,0,0,0,0,0 +units=m +no_defs"
r.external input="/home/mneteler/lsat7_2000.tif" band=1 output="rast_5c7bdac0189302" --overwrite -o
g.region n=228513.0 s=214975.5 e=645012.0 w=629992.5 res=28.5
r.series input=rast_5c7bdac0189302 method="average" range="0,0" output=output28566faed0e94fdaa03a9cfb57fb94a9 --overwrite

--> here range="0,0" must not be there unless the user enters values in the Processing form under "Advanced".

How to reproduce:

- test dataset (multilayer Landsat-7): lsat7_2000.tif attached (2MB)
- open dataset
- open r.series dialog from Processing
- select lsat7_2000.tif
- click "run" (default is "average" method which is fine to demonstrate the problem)

I tried to fix that but probably QgsProcessingParameterRange() is somehow broken or wrongly used in Processing?

It is likely that the suboptimal use of QgsProcessingParameterRange() affects also other GRASS GIS modules present in Processing.

lsat7_2000.tif - multilayer GeoTIFF Landsat-7 subscene (North Carolina sample dataset) (2.04 MB) Markus Neteler, 2019-03-03 03:36 PM


Related issues

Related to QGIS Application - Feature request #21558: Allow none/empty as possible values for Processing QgsPro... Open 2019-03-11

History

#1 Updated by Giovanni Manghi about 5 years ago

A quick and dirt solution would be set in the tool description file a different default. In 2.18 was something like -1000000,1000000, does it make sense?

#2 Updated by Markus Neteler about 5 years ago

Giovanni Manghi wrote:

A quick and dirt solution would be set in the tool description file a different default. In 2.18 was something like -1000000,1000000, does it make sense?

Well, sense not too much as it may lead to stealth bad results (where the range is outside of the workaround in 2.18 values but the user doesn't realize it).

We just fixed a similar case in r.neighbors, see https://github.com/qgis/QGIS/pull/9318
Couldn't that be adopted here as well to make "None" really functional for QgsProcessingParameterRange() as it does for QgsProcessingParameterNumber()?

#3 Updated by Giovanni Manghi about 5 years ago

  • Status changed from Open to Feedback

Well, sense not too much as it may lead to stealth bad results (where the range is outside of the workaround in 2.18 values but the user doesn't realize it).

We just fixed a similar case in r.neighbors, see https://github.com/qgis/QGIS/pull/9318
Couldn't that be adopted here as well to make "None" really functional for QgsProcessingParameterRange() as it does for QgsProcessingParameterNumber()?

yes I understand that would be the correct fix, but in the meantime we could set a less dangerous default than 0,0.

#4 Updated by Markus Neteler about 5 years ago

Giovanni Manghi wrote:

yes I understand that would be the correct fix, but in the meantime we could set a less dangerous default than 0,0.

Sure, better than nothing (= broken).

Will you change that (also in the 3.4 and 3.6 branches) or shall I do that?

#5 Updated by Giovanni Manghi about 5 years ago

  • Status changed from Feedback to Open
  • Priority changed from Normal to High
  • Crashes QGIS or corrupts data changed from Yes to No
  • Operating System deleted (Linux)
  • Assignee set to Giovanni Manghi

Will you change that (also in the 3.4 and 3.6 branches) or shall I do that?

I can do that

#6 Updated by Giovanni Manghi about 5 years ago

  • Category changed from GRASS to Processing/GRASS
  • Assignee deleted (Giovanni Manghi)
  • Affected QGIS version changed from 3.6.0 to 3.7(master)

Giovanni Manghi wrote:

Will you change that (also in the 3.4 and 3.6 branches) or shall I do that?

I can do that

Actually I can't. I gave a quick look at it with Luigi and it seems (we may be wrong) that is not possible to set any default value (other than 0,0) or set nothing as a default (which would allow to make this parameter behave really as optional, something that does not happen with 0,0).

#7 Updated by Jürgen Fischer almost 5 years ago

  • Related to Feature request #21558: Allow none/empty as possible values for Processing QgsProcessingParameterRange added

Also available in: Atom PDF