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

Provider quotes the arguments of the library() command - in full #111

Closed jfmoyen closed 1 year ago

jfmoyen commented 2 years ago

We have a script that runs a home-made library, I'm using here MASS as a quick MRE:

##exp=group
##Layer=vector
##Folder=string C:/

library(MASS)

# ... do interesting things

and works fine (in this case, of course it returns nothing so working 'fine' is relative).

However, if we modify the script like that

##exp=group
##Layer=vector
##Folder=string C:/

library(MASS,quietly=T)

# ... do interesting things

or in fact by adding any other option to library()

we get the following:

Error in library("MASS,quietly=T") : there is no package called 'MASS,quietly=T'

This seems to be because the parser takes the whole content of library() and quotes it:

R execution commands
....
tryCatch(find.package("sf"), error = function(e) install.packages("sf", dependencies=TRUE))
library("sf")
tryCatch(find.package("raster"), error = function(e) install.packages("raster", dependencies=TRUE))
library("raster")
tryCatch(find.package("MASS,quietly=T"), error = function(e) install.packages("MASS,quietly=T", dependencies=TRUE))
library("MASS,quietly=T")
JanCaha commented 2 years ago

I see the issue and understand why it is happening. The problem is that there the plugin needs to check if the package exist and install it otherwise. On the other hand, there is not much point in using quietly=T for scripts runned in QGIS. The text outcome of library load is not that disturbing and does not cause any issues.

It could be solved, by parsing library(MASS,quietly=T) to get package name and parameters, but I am slightly against that as it could potentially introduce a new set of issues.

jfmoyen commented 2 years ago

I was using MASS as an example of a library that everybody probably has, the actual script uses a home-made library that does many other things and for which it made sense to pass startup options. Point taken though.