north-road / qgis-processing-r

QGIS Processing R Provider Plugin
https://north-road.github.io/qgis-processing-r/
GNU General Public License v3.0
63 stars 14 forks source link

Optional enum literal returns a numeric variable #112

Closed gavg712 closed 2 years ago

gavg712 commented 2 years ago

I wanted to use an enum literal as optional, however the behavior is similar to an optional enum.

This is the script parameter line:

##Transform=optional enum literal boxcox;exp;log;log10;log1p;log2

The widget displays fine, but the execution returns

Transform <- 5

The advanced specification does not have the option to make it literal.

It will be nice to have an optional enum literal implementation or... Is there something I missed in the use of enum literal as optional?

JanCaha commented 2 years ago

The enum literal is a hack that i implemented to make it work. But it does not really support stuff like setting it optional. If you want that you have to go with normal enum and convert it to text in R code.

gavg712 commented 2 years ago

Yes, I knew it was a hack, but reviewing the code on line 305 of algorithm.py, it looks like it would be easy to have the "optional" version of your hack implementation....

if "=enum literal" in RUtils.upgrade_parameter_line(line):
            self.r_templates.add_literal_enum(value)

It might be enough to remove the = from the string that compares to the parameter line. I have tested it quickly on my local installation, but maybe you put that character for some good reason that I don't know. What do you think?

JanCaha commented 2 years ago

The problem is that the enum literal is processes in two steps. First the values are extracted into a variable and the enum is then processed as usual, but when it should be passed to the R script the string value is extracted from the values based on numeric value of the enum. Turning it to optional is likely to break the workflow and cause problems sooner or later. I dont think that it is good idea to try to add another level of complexity around this hack.

If you need the enum to be optional then should probably deal with it using classic enum and handle string values in the R script.

nyalldawson commented 2 years ago

Since a few qgis versions it's been possible to use a string parameter with a list of values to use for the user to pick from. Would that help address this need?

gavg712 commented 2 years ago

If you need the enum to be optional then should probably deal with it using classic enum and handle string values in the R script.

@JanCaha Yes, that is the way I handled this need in my scripts. This question started when my student used the optional enum literal parameter and did not understood why his script was not working. But having clarified the issue, perhaps I could just consider including a small note in the Syntax section of the web site.

Since a few qgis versions it's been possible to use a string parameter with a list of values to use for the user to pick from. Would that help address this need?

@nyalldawson That seems interesting, could you share more information about it? However, to me it is enough to use the classic enum has Jan mentioned.

Thanks guys