search

qgis / QGIS

QGIS is a free, open source, cross platform (lin/win/mac) geographical information system (GIS)
https://qgis.org
GNU General Public License v2.0
10.52k stars 2.99k forks source link

QGIS model fails if component_description contains a hyphen #48573

Open pathmapper opened 2 years ago

pathmapper commented 2 years ago

What is the bug or the crash?

QGIS model fails if component_description contains a hyphen (e.g. kamp-lintfort) and the component output is used in an expression (e.g. @kamp-lintfort_OUTPUT).

grafik

grafik

grafik

Steps to reproduce the issue

  1. Run the test model: test.zip
  2. See error: _Prepare algorithm: native:retainfields_1 Could not evaluate expression for parameter INPUT for Retain fields: Field 'lintfortOUTPUT' not found Execution failed after xxx seconds but the output variable offered and used is named @kamp-lintfort_OUTPUT grafik

Versions

QGIS-Version | 3.24.2-Tisler | QGIS-Codeversion | 13c1a028 -- | -- | -- | -- Qt-Version | 5.15.2 Python-Version | 3.9.5 GDAL-Version | 3.4.2 PROJ-Version | 9.0.0 EPSG-Registraturdatenbankversion | v10.054 (2022-02-13) GEOS-Version | 3.10.2-CAPI-1.16.0 SQLite-Version | 3.38.1 PDAL-Version | 2.3.0 PostgreSQL-Client-Version | unknown SpatiaLite-Version | 5.0.1 QWT-Version | 6.1.3 QScintilla2-Version | 2.11.5 BS-Version | Windows 10 Version 1809   |   |   |   Aktive Python-Erweiterungen db_manager | 0.1.20 grassprovider | 2.12.99 MetaSearch | 0.3.6 processing | 2.12.99 sagaprovider | 2.12.99 ### Supported QGIS version - [X] I'm running a supported QGIS version according to the roadmap. ### New profile - [X] I tried with a new QGIS profile ### Additional context If the hyphen is removed from the description text and variable name, the model runs fine ->[test_without_hyphen.zip](https://github.com/qgis/QGIS/files/8699804/test_without_hyphen.zip)
pathmapper commented 2 years ago

Looks like currently no variables with names containing a hyphen are supported because the hyphen is intepreted as subtraction operator, e.g. just open the field calculator for an arbitrary layer and type@one-two:

grafik

Pedro-Murteira commented 2 years ago

I can reproduce aswell, on windows 10 (QGIS 3.24.2).

DelazJ commented 2 years ago

Sounds like the variable naming bug of #46459 or #44379...

pathmapper commented 2 years ago

Issue confirmed with master (79f748c65d) on Ubuntu 20.04.4 LTS.

elpaso commented 2 years ago

I think I know from where this comes (the expression lexer) but I'm not sure if it is fixable (I think it's not) and if it needs to be fixed.

I would say that @kamp-lintfort_OUTPUT is NOT a valid variable name and the bug is in the algorithm that generate the variable names from the model step name.

pathmapper commented 2 years ago

There's already a logic to create a safe variable name:

https://github.com/qgis/QGIS/blob/0434469dd08409dda1846795015fcd7fa72bcab1/src/core/processing/models/qgsprocessingmodelalgorithm.cpp#L2183-L2193

so @kamp-lintfort_OUTPUT would become @kamplintfort_output.

But it looks like this takes effect only for algorithms of newly created models with QGIS >= 3.26 (ref https://github.com/qgis/QGIS/pull/47559, so failing also with master which is 3.25), so for 3.22 LTR and existing models this would still be an issue.

pathmapper commented 2 years ago

The question is whether this is something which could (and should) be fixed without breaking existing models or if we should just leave it like it is currently as this is fixed for new models created with QGIS >= 3.26.

The advice for existing models and QGIS < 3.26 would be to use only safe algorithm descriptions, i.e. only chars a-z_A-Z0-9 and don't start with a number.

pathmapper commented 2 years ago

I was wrong, the issue still exists with QGIS >= 3.26.

QgsProcessingModelAlgorithm::safeName is currently used for model input variables, not for variables related to a model algorithm.