rsbivand / rgrass

Interpreted interface between the GRASS geographical information system and R
https://rsbivand.github.io/rgrass/
24 stars 8 forks source link

Suggestion: wrapper function to pass full GRASS command syntax as a string #17

Closed florisvdh closed 3 years ago

florisvdh commented 3 years ago

In the past, I found myself using system() in Linux to pass one full GRASS command string (including flags & parameters) directly to the shell (sometimes a string of multiple command lines, or just the path of a shellscript containing a sequence of GRASS commands). It felt more natural with regard to the GRASS documentation, it was handy for copy-pasting statements between R and the shell, so it did not need the conversion to/from execGRASS() syntax. There may be pitfalls in that approach which I'm still unaware of, but at least some have become obvious:

While execGRASS() does not have those problems, some solution to solve the above problems was needed for the system() approach.

The following prototype function (which I'm starting to use) tries to solve this. I wonder whether a function like this would be a useful part of the rgrass7 package?

execshell <- function(commandstring, intern = FALSE) {
  if (.Platform$OS.type == "windows") {
    res <- shell(commandstring, intern = TRUE)
  } else {
    res <- system(commandstring, intern = TRUE)
  }
  if (!intern) cat(res, sep = "\n") else return(res)
}

If yes, I would be prepared to make a small pull request during one of coming weeks, but of course @rsbivand you may want to implement such functionality in a way that you deem (more) appropriate.

Regarding the above prototype, I'm not sure (yet) what exactly will happen with errors and warnings from GRASS. There may be differences between system() and shell() in that regard; they don't seem to share all arguments.

rsbivand commented 3 years ago

Basically definitely no. This is not helpful, as you choose to avoid all the checking/caching done in parseGRASS() and doGRASS(), which are called by execGRASS() to ensure that the string finally submitted is correct in terms of syntax and argument types.

You could think rather of a string function de-parsing a string like that you propose in order to construct the proper input to execGRASS(), which would not discard all the checking done in parseGRASS() and doGRASS(). The ouput of that function could then be run through execGRASS() if it succeeds.

florisvdh commented 3 years ago

OK, thanks for shedding light on the philosophy. I understand that my suggested approach does not implement the checks that are provided by rgrass7, and its philosophy (delegating that responsability to the user) would not fit the package. The idea of a string deparsing function is interesting indeed, but would need some more time (using regex with base R functions) if I'm to do it. As far as I'm concerned the issue can be closed, but when I find the time and if you're still interested, I would be interested to try.

rsbivand commented 3 years ago

Added after checking lightly, thanks. I've added you to DESCRIPTION as "ctb", hope that is OK.

florisvdh commented 3 years ago

Sure, thanks @rsbivand.

The regular expression to check the cmd part of string could be improved; I'll send a small PR for that (coming very soon).

rsbivand commented 3 years ago

OK, merged. When would you like this submitted to CRAN?

florisvdh commented 3 years ago

Thanks! Currently I have no elaborated new ideas in mind :wink: , so leaving the consideration about CRAN-submission to you.

florisvdh commented 3 years ago

Closing this issue - was fixed in #18.