opengeos / whiteboxR

WhiteboxTools R Frontend
https://whiteboxR.gishub.org
Other
172 stars 30 forks source link

relative paths in package options `whitebox.exe_path` and `whitebox.wd` #98

Closed brownag closed 1 year ago

brownag commented 1 year ago

Use of relative paths in whitebox.exe_path and whitebox.wd is supported. Relative paths are widely used as inputs to wbt_* functions.

But relative paths used for the WhiteboxTools executable or working directory can lead to unexpected behavior--especially if the user is using setwd()/getwd() or other R methods of managing working directories.

One example: "install_whitebox() into parent directory" works, but probably should result in an absolute whitebox.exe_path

library(whitebox)

install_whitebox("..")
#> Performing one-time download of WhiteboxTools binary from
#>   https://www.whiteboxgeo.com/WBT_Linux/WhiteboxTools_linux_amd64.zip 
#> (This could take a few minutes, please be patient...)
#> WhiteboxTools binary is located here:  ../WBT/whitebox_tools 
#> You can now start using whitebox
#>     library(whitebox)
#>     wbt_version()

wbt_exe_path(shell_quote = FALSE)
#> [1] "../WBT/whitebox_tools"

Similarly, setting working directory to a relative path wbt_wd(".") (or equivalent via wbt_init()) results in the relative path "."


There are times (such as when doing a fresh install) I would argue it makes most sense to return the absolute path for resulting executable.

Relative paths could be expanded with e.g. base::normalizePath() R.utils::getAbsolutePath(). I am considering a non-default option to opt-in to that, but need to investigate further. Another place this could be integrated would be functions for whitebox-specific .Renviron files #97

brownag commented 1 year ago

I opted to use normalizePath() rather than path.expand() only in the wbt_install() routine so that the configuration after install doesn't use relative paths even if the installation directory is specified as a relative path. Still performs tilde expansion as before.

I have not seen complaints about the relative paths set as EXE path, working directory, etc. I see no need to change this behavior for file paths in general. It is easy enough to avoid in most cases (e.g. "." v.s. getwd()) I tested reverse dependencies to see if there would be any obvious problems if a broader change to all absolute paths were made--and saw no problems--but that is not to say that folks don't rely on the current relative path behavior and/or generating "transferable" commands with command_only=TRUE passed to tools.