klmr / box

Write reusable, composable and modular R code
https://klmr.me/box/
MIT License
862 stars 48 forks source link

Push Box Path #300

Open jtlandis opened 1 year ago

jtlandis commented 1 year ago

Please describe your feature request

To allow for the default box path, retrieved by Sys.getenv("R_BOX_PATH") or getOptions("box.path"), to be prepended with additional paths.

Syntax may look something like

push_path <- function(path) {
    box_path <- Sys.getenv("R_BOX_PATH")
    if (!identical(box_path,"")) {
        path <- paste(path, collapse = ":")
        Sys.setenv(R_BOX_PATH=paste(path, box_path, sep = ":"))
    } else {
        box_path <- getOptions("box.path", default = NULL)
        options(box.path = c(path, box_path))
    }
}

box::push_path(here::here("box"))

I have started to modularize a bunch of code at my work. I like the concept of having R_BOX_PATH set for the production server we all work on. In the moments that I have a new git repository and I have some code that isn't appropriate for all users but should still be locally scoped to this repository, I would like a simple way to prepend the path.

This feature would likely not be used within modules (as I believe prepending a search path in one module may affect how other modules work), and be for interactive use or functional scripts.

I'm aware that box can modify the search path easily with ./ or ../, but I often find that this usage pattern is most reliant for scripts written as modules or are referencing other related modules. This usage pattern is less reliant for functional scripts scripts, whose relative location may change when organizing the project.

Being able to prepend the search path allows for functional scripts, who are only loading modules and not becoming a module, the ability to find the modules they need without having to fully qualify its relative location.

klmr commented 1 year ago

If I understand correctly such a function is only really necessary because the environment variable R_BOX_PATH completely overrides getOption('box.path'), am I seeing that right?

Because otherwise you could just write

options(box.path = c('YOUR_NEW_PATH', getOption('box.path'))

… in other words, there wouldn’t really be a need for a dedicated push_path function.

If so, it might make more sense for ‘box’ to change its behaviour so that R_BOX_PATH doesn’t overrride box.path, and instead both values get used (but presumably with the environment variable going first). — Thoughts?


A word of caution about using here::here: the ‘here’ package is quite opinionated about how R code is structured, and doesn’t really work in lots of real-world situations (e.g. shell scripts written in R). I recommend using box::file instead. This (intentionally) doesn’t always yield the same path as here::here, but it should work as a full replacement.

jtlandis commented 1 year ago

Your assessment is correct.

I personally like the idea of adding a .path argument to box::use that prepends the search path for only the modules in the current box::use call (and maybe not the submodules).

This feature request is really just to streamline a usage pattern. I could simply type

Sys.setenv(R_BOX_PATH=paste(path, Sys.getenv("R_BOX_PATH"), sep = ":"))

At the top of each of my scripts, but this looks ugly and I am also lazy 😅

jtlandis commented 1 year ago

Mistakenly closed the thread. 😓

jtlandis commented 1 year ago

I have just learned that you may set up a .Rprofile file in my repository and simply have a line that says.

Sys.setenv(R_BOX_PATH=paste(here::here(), Sys.getenv("R_BOX_PATH"), sep = ":"))

This should be sufficient to get the effect I would like.

klmr commented 1 year ago

I’ve reopened the issue since I am still looking into ways of consolidating the environment variable and R option, rather than have one override the other.

Regarding your solution: I generally advise against here::here() since it’s rather idiosyncratic in its way of determining the current path. box::file() should be a more robust solution in almost all situations (the only current exception that I am aware of is unfortunately code that is invoked via ‘callr’, and hopefully in the future this will also be fixed).

jtlandis commented 1 year ago

Thank you for your suggestion.

For context, I would only use here::here() for projects in which the code is self contained to that project only and may not be reused. here::here() would definitely not work if I wanted to call a module in that project from another project.

I think box::file() would make more sense if the .Rprofile document was considered a module. This document however is automatically sourced at the start of a Session which is not the same as using box::use(), thus box::file() would just print your current working directory.

a hacky solution could be setting up a file in the same directory as .Rprofile like a .box which contains

#' @export
file <- box::file()

and in the .Rprofile do something like

box::use(./.box)
Sys.setenv(R_BOX_PATH=paste(.box$file, Sys.getenv("R_BOX_PATH"), sep = ":"))

But again, requiring ./.box makes the working directory relevant when sourcing the .Rprofile


The closest thing to "working" I could get would be the following (i.e. if I were to source a .Rprofile from outside of the project)

.where <- sys.calls()[[1]]
if (is.null(.where)) {
  .file <- getwd()
} else if (!is.null(names(.where))) {
  .file <- normalizePath(dirname(.where[[which(names(.where)=="file")]]), winslash = "/")
} else {
  .file <- where[[2]]
}
Sys.setenv(R_BOX_PATH=paste(.file, Sys.getenv("R_BOX_PATH"), sep = ":"))
jtlandis commented 1 year ago

but the above is probably outside the scope of your current reasons for keeping the issue open. Hope this helps!

klmr commented 1 year ago

thus box::file() would just print your current working directory.

That’s right (in this case). But isn’t the working directory precisely what you want? After all, a project-specific .Rprofile file is only sourced when it’s sitting in the current working directory. Or is there a scenario in which a project’s .Rprofile file is sourced even though the R is being run from a different directory?

jtlandis commented 1 year ago

Forgive me, I was just trying to think of scenarios of why I would not use here::here(), which looks for the nearest .Rproj file. For my use case in this issue here::here() and box::file() will yield the same result.

I currently cannot think of an example where we would source a .Rprofile outside a directory. I suppose I was just thinking up abstract hedge cases 😅