I am using Windows 10 with Python 3.9.10 and rpy2 3.5.2. Here is my `rpy2.situation` output. R is install from CRAN including RStudio.
``` C:\Users\user>py -3 -m rpy2.situation rpy2 version: 3.5.2 Python version: 3.9.10 (tags/v3.9.10:f2f3f53, Jan 17 2022, 15:14:21) [MSC v.1929 64 bit (AMD64)] Looking for R's HOME: Environment variable R_HOME: None InstallPath in the registry: C:\Program Files\R\R-4.1.2 Environment variable R_USER: None Environment variable R_LIBS_USER: None Unable to determine R home: [WinError 2] Das System kann die angegebene Datei nicht finden Unable to determine R library path: Command '('C:\Program Files\R\R-4.1.2\bin\Rscript', '-e', 'cat(Sys.getenv("LD_LIBRARY_PATH"))')' returned non-zero exit status 1. R version: In the PATH: R version 4.1.2 (2021-11-01) -- "Bird Hippie" Loading R library from rpy2: OK Additional directories to load R packages from: None C extension compilation: Der Befehl "sh" ist entweder falsch geschrieben oder konnte nicht gefunden werden. Warning: Unable to get R compilation flags. ```
This is not only an info about my environment but also gives a hint about the problem. Look at the conflicting informations about "R HOME".
# Problem from user perspective The user get the information that `InstallPath` is there but `R HOME` not. Isn't that the same? I am not sure. The first is used on Windows the latter on unixoid systems?
# Technical perspective Let's reproduce the problem
``` Python 3.9.10 (tags/v3.9.10:f2f3f53, Jan 17 2022, 15:14:21) [MSC v.1929 64 bit (AMD64)] on win32 Type "help", "copyright", "credits" or "license" for more information.
import rpy2.situation rpy2.situation.get_r_home()
Unable to determine R home: [WinError 2] Das System kann die angegebene Datei nicht finden 'C:\Program Files\R\R-4.1.2' ```
Here you see a "heavy" error in the first output line and then just a "good" path in the second output line. The user doesn't understand that this are two trais where the frist failed and the second succeeded. So at the end is everything OK. But the user doesn't know that there is no problem anymore and that the error from first line isn't important anymore and solved by the "second line".
With adding some debug-print-code I found out that `r_home_from_subprocess()` is called first (and fails) and then `r_home_from_registry()` is called and succeed. That combination leads me to the wrapping function [`rpy2.situation.get_r_home()`](https://github.com/rpy2/rpy2/blob/8904d0c03ded74bfeedcab5c94c652df0d1a42a2/r...).
``` def get_r_home() -> Optional[str]: r_home = os.environ.get('R_HOME')
if not r_home: r_home = r_home_from_subprocess() if not r_home and os.name == 'nt': r_home = r_home_from_registry() logger.info(f'R home found: {r_home}') return r_home ```
I don't understand all details so please let me ask a question. Is there a difference between the first line (`os.environ.get('R_HOME')`) of that method and that what happens in `r_home_from_subprocess()` (calls `R R_HOME` in shell)? I don't see a difference and because of that the code doesn't make sense (to me).
I am not sure about a good solution. But I want to suggest some points.
1. Don't use `os` instead of [`plattform`](https://docs.python.org/library/platform.html) to determine the current operating system. 2. (Not my favorite) Add an `log_excpetion` (default `True`) argument to `r_home_from_subprocess()` to optionally deactivate the log output when called from `get_r_home()`. 3. Remove all log output from `r_home_from_subprocess()` and `r_home_from_registry()`. The functions should raise an Exception (does rpy2 has its own Exceptions classes?). The quality of that solution depends on from where and how many other code locations this two functions are called. If the code search of GitHub is correct they are called only in `get_r_home()`.
If you can help with finding and deciding about a solution I would create a PR including some refactoring of that three functions. Are there any test units for that functions?
To summarize: I suggest to remove log output from the two sub functions and let them raise Exceptions. Handle that exceptions in the wrapping `get_r_home()`. If they are there improve the unittests.
Wrong repository. ;)
Closed #3215 as completed.
github-comments@lists.geany.org