Bug report #21452
r.series broken in Processing due to wrong "range=0,0" pre-definition
| 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.
Related issues
History
#1
Updated by Giovanni Manghi over 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 over 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 over 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 over 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 over 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 over 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 over 5 years ago
- Related to Feature request #21558: Allow none/empty as possible values for Processing QgsProcessingParameterRange added