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

box::use searches wrong path when sourced from file in RStudio #225

Closed
klmr opened this issue Aug 1, 2021 · 9 comments
Closed

box::use searches wrong path when sourced from file in RStudio #225

klmr opened this issue Aug 1, 2021 · 9 comments

Comments

@klmr
Copy link
Owner

klmr commented Aug 1, 2021

Via Stack Overflow:

I previously tried box::use(./Resources/Modules/time), without using setwd(this.path::this.dir()), but it failed for me within RStudio. Below is the error message:

Error in mapply(FUN = f, ..., SIMPLIFY = FALSE) : Unable to load module “./Resources/Modules/time”; not found in “C:/Users/greg/OneDrive - My Company/Documents” (inside “find_in_path(spec, mod_search_path(caller))”)

[The box::use()] code is indeed inside an R source file Script.R; this file is open in RStudio; and I have executed it via source() (and Ctrl+Return too). However, that file is not stored in the Documents folder. Rather, it is stored in the Template (01) directory nested under my Desktop; Template (01) also contains the subdirectory Resources, which contains the subdirectory Modules, which contains time.R. The full location of the folder is: C:Users\greg\OneDrive - My Company\Desktop\Projects\Automations\Templates\Template (01)

@klmr klmr added the ⚠️ bug label Aug 1, 2021
@klmr
Copy link
Owner Author

klmr commented Aug 1, 2021

The question is: why is box::use searching the path C:/Users/greg/OneDrive - My Company/Documents instead of C:\Users\greg\OneDrive - My Company\Desktop\Projects\Automations\Templates\Template (01)?

@GregYannes
Copy link

GregYannes commented Aug 1, 2021

Correct, that is indeed the question!

To be clear, this not the global module search path, since box.path will only ever be set to a universally accessible location on a shared drive or server. Rather, it is the relative search path for project-specific modules, which fall within the \Resources\Modules subdirectory of that project; here
C:\Users\greg\OneDrive - My Company\Desktop\Projects\Automations\Templates\Template (01)\Resources\Modules.

I can say with confidence that the working directory was C:/Users/greg/OneDrive - My Company/Documents, when I ran getwd() from the console before source()ing Script.R in RStudio; and also that setwd(this.path::this.dir()) worked as intended at the beginning of Script.R.

While I haven't had a chance to fire up my work computer, I suspect that RStudio defaults to my Documents folder. That's why I am generally careful to "recalibrate" the working directory to this.path::this.dir() at the start of each script.

@GregYannes
Copy link

GregYannes commented Aug 4, 2021

Have you considered incorporating the this.path package into box as a one of its imports, and then availing yourself of the this.path::this.path() function?

This could be used in default behavior for box::set_script_path(), such that the path parameter defaults to either

  • this.path::this.path() whenever there is an R document in play
    or
  • NULL whenever there is no R document in play, in which case this.path::this.path() will throw an error like
Error in this.path::this.path() : 
  'this.path' used in an inappropriate fashion
* no appropriate source call was found up the calling stack
* R is being run from RStudio with no documents open

Alternatively, you could automatically attempt this call to this.path::this.path(), whenever the user makes a local call to project-specific modules:

box::use(./relative/path/to/local/module)

Then again, I fully understand if you want to limit dependency on external packages that (unlike base and tools) do not come with R.

@klmr
Copy link
Owner Author

klmr commented Aug 4, 2021

Have you considered incorporating the this.path package into box as a one of its imports, and then availing yourself of the this.path::this.path() function?

That isn’t a bad idea by any means. But ‘box’ already implements (some of) the same functionality, so it shouldn’t need ‘this.path’ (it implements it differently, but for reasons that shouldn’t matter here). Anyway, I’ve just now found out why ‘box’ fails here so I should have a fix soon. I also found another (unrelated) bug in the functionality, so this was worthwhile!

Apart from the above: as you’ve guessed, keeping ‘box’ free of dependencies is an important goal, since it’s an “infrastructure package”, so its installation should both be as easy as possible, and not depend on other things that might break.

@GregYannes
Copy link

GregYannes commented Aug 4, 2021

Great to know! Thank you for your transparency and attentiveness, Konrad!

I do have one further question: is there a way to permanently set the box.path option? Much like some options that do not come already baked into one's .Rprofile, the box.path does not persist from session to session. While some options might fail to persist in value, and thus "reset" their values with every new session, it appears that box.path will not even exist after terminating R, when getOption("box.path") returns NULL.

Ideally, one might wish to set box.path once and for all, as with .libPaths(). Do you think this is possible? If so, is this a topic for a whole new thread?

Thanks again! — Greg

@klmr
Copy link
Owner Author

klmr commented Aug 4, 2021

I’m not sure I understand … (why) can’t you persist the option inside your .Rprofile? After all, that’s where you’d normally customise other options and potentially .libPaths as well.

Alternatively, and just like the package library path, you can use an environment variable to set the path, R_BOX_PATH. This variable can be set either for your whole terminal session (e.g. in ~/.bashrc or ~/.profile) or for R only, inside your .REnviron file.
For more information, see the documentation, box::use: Search paths and more generally the R documentation about R startup.

@GregYannes
Copy link

GregYannes commented Aug 4, 2021

Thanks, I'll check out those links!

I was merely hoping there might be functionality within box to achieve the same effect. If I recall, when I used the bigrquery package, it cached some of the oath configuration permanently, so I didn't have to reconfigure everything after starting a new session. Then again, this might be significantly more complicated than I imagine, and I can always do it by hand.

@klmr
Copy link
Owner Author

klmr commented Aug 4, 2021

Hmm I think we might be talking past each other. The way I mentioned definitely allows to persist this configuration “permanently”, effortlessly, and I’m fairly sure that this is the intended way as far as R is concerned. Of course ‘box’ could maintain its own configuration file and variables instead of using R’s provide options but to what end?

(If I understand the ‘bigrquery’ use-case correctly then it’s different because it stores credentials, which are potentially sensitive user secrets, so they shouldn’t necessarily reside in the regular .Rprofile, which might be under version control or otherwise shared.)

Or are you looking for a function inside ‘box’ which merely writes the configuration to the user’s .Rprofile? I.e. something along these lines (omitting any error checking and sanitisation):

save_module_path = function (path) {
    init_code = sprintf('options(box.path = %s)', deparse(path))
    writeLines(init_code, Sys.getenv('R_PROFILE_USER'))
}

… to be honest I don’t see the value of such a function because it automates a process that’s already trivial (= open the .Rprofile file in an editor, add the desired code), and implementing it correctly is actually quite complex (e.g. users might prefer writing to .REnviron, and they might want to choose between global and user-specific configuration) and brittle (at minimum the code above should check whether the file in question actually exists, and needs to guard against including the configuration repeatedly if the user calls the function more than once). Lastly, CRAN does not like packages which write to user configuration files: such submission are automatically flagged and always need to be manually checked and green-lit. This causes submission delays and work for the CRAN maintainers.

@klmr klmr closed this as completed in cb2ef33 Aug 4, 2021
@GregYannes
Copy link

GregYannes commented Aug 5, 2021

Thank you for this detailed explanation! I certainly appreciate your reasoning, and I'm very inclined to agree.

Regarding your statement

To be honest I don’t see the value of such a function because it automates a process that’s already trivial (= open the .Rprofile file in an editor, add the desired code)

the only reason I wanted such a function is that these scripts, which employ box to access my modules, are ultimately executed by a VM. So I would love for everything to be handled once and for all, in as portable and "hands-off" a manner as possible (ie. by a single call to save_module_path() within a script).

However, given the complications, I should probably just do it manually.

Thanks again! — Greg

klmr added a commit that referenced this issue Aug 5, 2021
klmr added a commit that referenced this issue Aug 5, 2021
* Fix loading inside RStudio. Fixes #225.
* Work around R bug with spaces in paths on Unix
radbasa pushed a commit to Appsilon/box that referenced this issue Jul 1, 2024
* Fix loading inside RStudio. Fixes klmr#225.
* Work around R bug with spaces in paths on Unix
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