posit-dev / positron

Positron, a next-generation data science IDE
https://positron.posit.co
Other
2.62k stars 79 forks source link

Understanding `box` modules directory context #5082

Closed GitHunter0 closed 1 week ago

GitHunter0 commented 1 week ago

System details:

Positron and OS details:

Positron Version: 2024.11.0 (user setup) build 49 Code - OSS Version: 1.93.0 Commit: 10183fa8d9a8495b5d52cafd252cfa5a44b05599 Date: 2024-10-15T02:46:25.016Z Electron: 30.4.0 Chromium: 124.0.6367.243 Node.js: 20.15.1 V8: 12.4.254.20-electron.0 OS: Windows_NT x64 10.0.19045

Interpreter details:

R version 4.4.1 (2024-06-14 ucrt) Platform: x86_64-w64-mingw32/x64 Running under: Windows 10 x64 (build 19045)

Describe the issue:

Hi folks / @klmr , please consider the MWE below.

We have module1.R, module2.R and __initi__.R files, all in directory C:/project/modules/, with C:/project/ being our R working directory.

# module1.R
f1 <- function(x) return(x)

In RStudio, running box::use(./module1[f1]) interactively from module2.R works as expected.

However, in Positron, it generates this error:

Error in `box::use()`:
! unable to load module “./module1”; not found in "C:/project/"

Which means Positron does not understand we are running a code from a box module and therefore it should consider its directory as the reference, not the R working directory.

Thank you

DavisVaughan commented 1 week ago

I don't think Positron or RStudio "understand" anything about box, so this is likely somehow an issue with box itself. Maybe @klmr can provide more insight though

klmr commented 1 week ago

@DavisVaughan Correct; since R has no reliable way (!) of determining the currently executing script’s path, ‘box’ needs to hard-code support for each individual launcher. For RStudio, the relevant logic can be found in R/paths.r#L182-L220.

Positron will need to expose a similar R API so that ‘box’ can support it.

@GitHunter0 Could you please create a new feature request for ‘box’?

GitHunter0 commented 1 week ago

For sure @klmr , just opened https://github.com/klmr/box/issues/374 .

DavisVaughan commented 1 week ago

@klmr could you use rstudioapi::isAvailable() instead of checking for .Platform$GUI %!=% 'RStudio'? We've made it so that does return TRUE in Positron.

rstudioapi::getActiveDocumentContext() will also work too when rstudioapi is installed.

The internal API fallback you are using there will not work though, as that is not officially supported

klmr commented 1 week ago

@DavisVaughan

There are unfortunately two issues with this approach:

  1. rstudioapi::getActiveDocumentContext() can also be invoked outside RStudio (e.g. when running a script in the terminal). In those cases it throws an error. I am using the .Platform$GUI test to distinguish these scenarios. I could use rstudioapi::isAvailable() instead, but that performs the same test.
  2. ‘rstudioapi’ might (perhaps surprisingly!) not be installed when running inside RStudio (think ‘renv’ project library), which is why I included a fallback via as.environment('tools:rstudio') if .Platform$GUI is set to 'RStudio'. Would that environment be attached inside Positron as well?
klmr commented 1 week ago

Okay, I’ve tried it, and there’s yet another issue:

  1. In Positron, rstudioapi::getActiveDocumentContext() returns the wrong value when run from the console; namely, it returns the path of the file in the currently active document tab (or NULL if no file is open), even though that file isn’t where the code is being run from.
DavisVaughan commented 1 week ago

I could use rstudioapi::isAvailable() instead, but that performs the same test.

In Positron we override rstudioapi::isAvailable() to do something different, so it is not performing exactly the same code that the one from the actual rstudioapi package is using - i.e. it is not checking .Platform$GUI

‘rstudioapi’ might (perhaps surprisingly!) not be installed when running inside RStudio

To me, if rstudioapi is not installed, you cannot perform that action, full stop. That is the only official way to access that IDE support. Going through internal means to access it when in-rstudio-but-rstudioapi-is-not-installed is not something we can support

DavisVaughan commented 1 week ago

Is 3 such a blocker right now? I feel like most people don't call that directly. Is there a use case I'm missing?

klmr commented 1 week ago

3 might not be a blocker, but it would be nice if that worked. People definitely do use the terminal directly.

I’ll use rstudioapi:isAvailable() for now. If it does something different in Positron that might work.

That said, I’ll keep the fallback for RStudio since that situation is actually very common: at least in every ‘renv’ project that I’ve worked with (nobody installs IDE utilities in their ‘renv’, nor should they; it could be configured via RENV_CONFIG_EXTERNAL_LIBRARIES, but that require extra work and knowledge on behalf of the user, and I’d wager that it isn’t done on 99% of installations).