Use reticulate's py_require() to manage the Python installation and environment#82
Use reticulate's py_require() to manage the Python installation and environment#82
Conversation
- Change to the new suggested `py_require()` functions from *reticulate* (issue #81).
- Update the README pointing to the new configuration. - Update the installation instruction vignette.
- Add tolerance to spectrum_utils rtime comparison. - Update information in vignette about Python configuration. - Check environment variables on package loading to display warnings.
vignettes/SpectriPy.qmd
Outdated
| @sec-python for options to skip this and use either *miniconda* or system Python | ||
| environment instead. | ||
| `install.packages("BiocManager")`. As a system dependency, the package requires | ||
| Python (version >= 2.12) to be available. During package installation, |
There was a problem hiding this comment.
3.12, no idea >= or > (in read me it is ">")
There was a problem hiding this comment.
Thanks! it's >= 3.12 - so I'll also fix in README.
vignettes/SpectriPy.qmd
Outdated
|
|
||
| 1) Check the output of `Sys.getenv()`: is there a system variable | ||
| `RETICULATE_PYTHON` or `RETICULATE_PYTHON_ENV` defined? Problem: *reticulate* | ||
| will by use the specified Python or Python environment and skip automatic |
vignettes/SpectriPy.qmd
Outdated
| evaluate if the required Python libraries (i.e. *matchms* version 0.28.2 and | ||
| *spectrum_utils* version 0.3.2-0) are available. If they are not available, | ||
| evaluate if the required Python libraries (i.e., *matchms* version 0.30.0 and | ||
| *spectrum_utils* version 0.3.2) are available. If they are not available, |
There was a problem hiding this comment.
maybe write the numpy version too, for completeness?
DESCRIPTION
Outdated
| @@ -53,7 +53,7 @@ Authors@R: c(person(given = "Michael", family = "Witting", | |||
| ) | |||
| Depends: | |||
| R (>= 4.1.0), | |||
There was a problem hiding this comment.
aaah, good point. I'll update that.
vignettes/SpectriPy.qmd
Outdated
| *spectrum_utils* version 0.3.2) are available. If they are not available, | ||
| *SpectriPy* will install them using functionality from the *reticulate* R | ||
| package (see also | ||
| package, in particular, *reticulate*'s `py_require()` function (see also |
There was a problem hiding this comment.
I would leave out this 1 sentence regarding py_require(). Clear with "note" and "introduction" sections above.
mdgrv
left a comment
mdgrv
left a comment
There was a problem hiding this comment.
Very clearly explained the big changes. See smaller suggestions in single comments. The bigger suggestion (but you can leave it as such too), is to move the 2 appendix notes "sec-python" and "sec-fix" under appendices in the vignette: "detailed-installation-configuration.qmd". The reason is, I don't expect difficulties for 1st time installers (section Installation), as everything is installed in the background for them. if you want something more advanced, you go to the detailed install vignette.
|
Thank you so much for the review @mdgrv ! I will also move the two sections to the installation vignette as you suggested - makes more sense to have all configuration and installation related information in there. |
2 participants
This PR changes from the manual conda/virtualenv-based Python library installation to the newer, and recommended
reticulate::py_require()-based approach (see also issue #81). This simplifies the code considerably.Installation documentation was updated accordingly and a new troubleshooting section added (based on experiences and issues described in issue #81 ).