Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Push Box Path #300

Open
jtlandis opened this issue Oct 23, 2022 · 9 comments
Open

Push Box Path #300

jtlandis opened this issue Oct 23, 2022 · 9 comments

Comments

@jtlandis
Copy link

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
Copy link
Owner

klmr commented Oct 28, 2022

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
Copy link
Author

jtlandis commented Oct 28, 2022

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 jtlandis reopened this Oct 28, 2022
@jtlandis
Copy link
Author

Mistakenly closed the thread. 😓

@jtlandis
Copy link
Author

jtlandis commented Dec 9, 2022

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.

@jtlandis jtlandis closed this as completed Dec 9, 2022
@klmr klmr reopened this Dec 11, 2022
@klmr
Copy link
Owner

klmr commented Dec 11, 2022

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
Copy link
Author

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
Copy link
Author

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

@klmr
Copy link
Owner

klmr commented Dec 11, 2022

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
Copy link
Author

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 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants