From 281a6b4d3d3b60efc231475a260db5db2892ee4a Mon Sep 17 00:00:00 2001 From: Tomasz Kalinowski Date: Sat, 22 Feb 2025 10:04:04 -0500 Subject: [PATCH 1/7] pass through uv stdout too; contains progress on windows --- R/py_require.R | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/R/py_require.R b/R/py_require.R index aab09308..8f6fc94b 100644 --- a/R/py_require.R +++ b/R/py_require.R @@ -617,6 +617,8 @@ uv_get_or_create_env <- function(packages = py_reqs_get("packages"), if (is_positron()) c(RUST_LOG = NA) )) + uv_output_file <- tempfile() + on.exit(unlink(uv_output_file), add = TRUE) uv_args <- c( "run", @@ -627,17 +629,20 @@ uv_get_or_create_env <- function(packages = py_reqs_get("packages"), exclude_newer, packages, "--", - "python", "-c", "import sys; print(sys.executable);" + "python", "-c", + # chr(119) == "w", but avoiding a string literal to minimize the need for + # shell quoting shenanigans + "import sys; f=open(sys.argv[-1], chr(119)); f.write(sys.executable); f.close();", + uv_output_file ) # debug print system call - if (Sys.getenv("_RETICULATE_DEBUG_UV_") == "1") + if (debug <- Sys.getenv("_RETICULATE_DEBUG_UV_") == "1") message(paste0(c(shQuote(uv), maybe_shQuote(uv_args)), collapse = " ")) - env_python <- suppressWarnings(system2(uv, maybe_shQuote(uv_args), stdout = TRUE)) - error_code <- attr(env_python, "status", TRUE) + error_code <- suppressWarnings(system2(uv, maybe_shQuote(uv_args))) - if (!is.null(error_code)) { + if (error_code) { cat("uv error code: ", error_code, "\n", sep = "", file = stderr()) msg <- do.call(py_reqs_format, call_args) writeLines(c(msg, strrep("-", 73L)), con = stderr()) @@ -650,7 +655,10 @@ uv_get_or_create_env <- function(packages = py_reqs_get("packages"), stop("Call `py_require()` to remove or replace conflicting requirements.") } - env_python + ephemeral_python <- readLines(uv_output_file, warn = FALSE) + if (debug) + message("resolved ephemeral python: ", ephemeral_python) + ephemeral_python } #' uv run tool From ce481840d521df753e6b97c61199ae4e7c0281d4 Mon Sep 17 00:00:00 2001 From: Tomasz Kalinowski Date: Sat, 22 Feb 2025 10:06:18 -0500 Subject: [PATCH 2/7] fix installer on windows - Don't produce file named `Out-Null` in cwd - Handle spaces, mixed \ and / in paths. --- R/py_require.R | 33 ++++++++++++++++++--------------- R/utils.R | 2 +- 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/R/py_require.R b/R/py_require.R index 8f6fc94b..52bc42c1 100644 --- a/R/py_require.R +++ b/R/py_require.R @@ -549,6 +549,7 @@ uv_binary <- function(bootstrap_install = TRUE) { if (bootstrap_install) { # Install 'uv' in the 'r-reticulate' sub-folder inside the user's cache directory # https://github.com/astral-sh/uv/blob/main/docs/configuration/installer.md + dir.create(dirname(uv), showWarnings = FALSE, recursive = TRUE) file_ext <- if (is_windows()) ".ps1" else ".sh" url <- paste0("https://astral.sh/uv/install", file_ext) install_uv <- tempfile("install-uv-", fileext = file_ext) @@ -557,25 +558,27 @@ uv_binary <- function(bootstrap_install = TRUE) { return(NULL) # stop("Unable to download Python dependencies. Please install `uv` manually.") } + if(debug <- Sys.getenv("_RETICULATE_DEBUG_UV_") == "1") + system2 <- system2t if (is_windows()) { - system2("powershell", c( - "-ExecutionPolicy", "ByPass", - "-c", - paste0( - "$env:UV_UNMANAGED_INSTALL='", dirname(uv), "';", # shQuote()? - # 'Out-Null' makes installation silent - "irm ", install_uv, " | iex *> Out-Null" + + withr::with_envvar(c("UV_UNMANAGED_INSTALL" = shortPathName(dirname(uv))), { + system2("powershell", c( + "-ExecutionPolicy", "ByPass", "-c", + sprintf("irm %s | iex", shortPathName(install_uv))), + stdout = if (debug) "" else FALSE, + stderr = if (debug) "" else FALSE ) - )) - } else if (is_macos() || is_linux()) { + }) + + } else { + Sys.chmod(install_uv, mode = "0755") - dir.create(dirname(uv), showWarnings = FALSE, recursive = TRUE) - system2(install_uv, c("--quiet"), - env = c( - paste0("UV_UNMANAGED_INSTALL=", maybe_shQuote(dirname(uv))) - ) - ) + withr::with_envvar(c("UV_UNMANAGED_INSTALL" = dirname(uv)), { + system2(install_uv, c(if (!debug) "--quiet")) + }) + } } diff --git a/R/utils.R b/R/utils.R index e788e133..3a048849 100644 --- a/R/utils.R +++ b/R/utils.R @@ -687,7 +687,7 @@ reticulate_cache_dir <- function(...) { path.expand(rappdirs::user_cache_dir("r-reticulate", NULL)) } - file.path(root, ...) + normalizePath(file.path(root, ...), mustWork = FALSE) } From 1b68c8e4146f6464cc797f570f2895d677a5789c Mon Sep 17 00:00:00 2001 From: Tomasz Kalinowski Date: Sat, 22 Feb 2025 10:12:50 -0500 Subject: [PATCH 3/7] internal utils --- R/py_require.R | 9 +++++++++ R/utils.R | 18 +++++++++--------- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/R/py_require.R b/R/py_require.R index 52bc42c1..2201d135 100644 --- a/R/py_require.R +++ b/R/py_require.R @@ -752,6 +752,15 @@ uv_python_list <- function(uv = uv_binary()) { x } +uvx_binary <- function(...) { + uv <- uv_binary(...) + if(is.null(uv)) { + return() + } + uvx <- file.path(dirname(uv), if (is_windows()) "uvx.exe" else "uvx") + if (file.exists(uvx)) uvx else NULL # print visible +} + resolve_python_version <- function(constraints = NULL, uv = uv_binary()) { constraints <- as.character(constraints %||% "") constraints <- trimws(unlist(strsplit(constraints, ",", fixed = TRUE))) diff --git a/R/utils.R b/R/utils.R index 3a048849..3e60cbc7 100644 --- a/R/utils.R +++ b/R/utils.R @@ -647,15 +647,15 @@ maybe_shQuote <- function(x) { rm_all_reticulate_state <- function(external = FALSE) { rm_rf <- function(...) - unlink(path.expand(c(...)), recursive = TRUE, force = TRUE) + try(unlink(path.expand(c(...)), recursive = TRUE, force = TRUE)) if (external) { if (!is.null(uv <- uv_binary(FALSE))) { system2(uv, c("cache", "clean")) - rm_rf(system2(uv, c("python", "dir"), - env = "NO_COLOR=1", stdout = TRUE)) - rm_rf(system2(uv, c("tool", "dir"), - env = "NO_COLOR=1", stdout = TRUE)) + withr::with_envvar(c("NO_COLOR"="1"), { + rm_rf(system2(uv, c("python", "dir"), stdout = TRUE)) + rm_rf(system2(uv, c("tool", "dir"), stdout = TRUE)) + }) } if (nzchar(Sys.which("pip3"))) @@ -670,12 +670,12 @@ rm_all_reticulate_state <- function(external = FALSE) { rm_rf(virtualenv_path("r-reticulate")) for (venv in virtualenv_list()) { if (startsWith(venv, "r-")) - virtualenv_remove(venv, confirm = FALSE) + rm_rf(virtualenv_path(venv)) } rm_rf(reticulate_cache_dir()) - try(tools::R_user_dir("reticulate", "cache")) - try(tools::R_user_dir("reticulate", "data")) - try(tools::R_user_dir("reticulate", "config")) + rm_rf(tools::R_user_dir("reticulate", "cache")) + rm_rf(tools::R_user_dir("reticulate", "data")) + rm_rf(tools::R_user_dir("reticulate", "config")) invisible() } From ff0bd20413b8d6b99ca2a64a626050782933c5a6 Mon Sep 17 00:00:00 2001 From: Tomasz Kalinowski Date: Sat, 22 Feb 2025 11:05:39 -0500 Subject: [PATCH 4/7] fix finalizer test on windows - handle filepaths with \ by passing constructing a raw string literal python expression --- tests/testthat/test-finalize.R | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/testthat/test-finalize.R b/tests/testthat/test-finalize.R index 2f320c83..9184dc08 100644 --- a/tests/testthat/test-finalize.R +++ b/tests/testthat/test-finalize.R @@ -14,12 +14,12 @@ class Foo: weakref.finalize(self, self.on_finalize) def on_finalize(self): - with open('%s', 'a') as f: + with open(r'%s', 'a') as f: f.write('Foo.finalize ran\\n') import atexit def on_exit(): - with open('%s', 'a') as f: + with open(r'%s', 'a') as f: f.write('on_exit finalizer ran\\n') atexit.register(on_exit) From 96b5dc529c24de658c268e7007182ab6a33e70ed Mon Sep 17 00:00:00 2001 From: Tomasz Kalinowski Date: Sat, 22 Feb 2025 12:59:10 -0500 Subject: [PATCH 5/7] test fixes on windows --- src/python.cpp | 3 ++- tests/testthat/test-interrupts.R | 9 ++++++--- tests/testthat/test-python-arrays.R | 2 ++ 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/python.cpp b/src/python.cpp index 9aabd5e2..747b4427 100644 --- a/src/python.cpp +++ b/src/python.cpp @@ -2832,6 +2832,7 @@ PyOS_sighandler_t install_interrupt_handlers_() { // the correct handler. // First, install the Python handler + GILScope _gil; PyObject *main = PyImport_AddModule("__main__"); // borrowed ref PyObject *main_dict = PyModule_GetDict(main); // borrowed ref PyObjectPtr locals(PyDict_New()); @@ -2867,7 +2868,7 @@ PyObject* python_interrupt_handler(PyObject *module, PyObject *args) // it sees that trip_signals() had been called. // args will be (signalnum, frame), but we ignore them - + GILScope _gil; if (R_interrupts_pending == 0) { // R won the race to handle the interrupt. The interrupt has already been // signaled as an R condition. There is nothing for this handler to do. diff --git a/tests/testthat/test-interrupts.R b/tests/testthat/test-interrupts.R index 162154b4..0f50e9c6 100644 --- a/tests/testthat/test-interrupts.R +++ b/tests/testthat/test-interrupts.R @@ -6,7 +6,10 @@ test_that("Running Python scripts can be interrupted", { time <- import("time", convert = TRUE) # interrupt this process shortly - system(paste("sleep 1 && kill -s INT", Sys.getpid()), wait = FALSE) + interruptor <- callr::r_bg(args = list(pid = Sys.getpid()), function(pid) { + Sys.sleep(1) + ps::ps_interrupt(ps::ps_handle(pid)) + }) # tell Python to sleep before <- Sys.time() @@ -70,7 +73,7 @@ test_that("interrupts work when Python is running", { p$wait() expect_identical(p$get_exit_status(), 0L) - expect_identical(p$read_output(), "Caught interrupt; Finished!") + expect_identical(p$read_all_output(), "Caught interrupt; Finished!") }) @@ -124,7 +127,7 @@ test_that("interrupts can be caught by Python", { expect_identical(p$get_exit_status(), 0L) expect_identical( - p$read_output(), + p$read_all_output(), "Caught interrupt; Running finally; Python finished; R Finished!") }) diff --git a/tests/testthat/test-python-arrays.R b/tests/testthat/test-python-arrays.R index dc5e2614..3b4409e2 100644 --- a/tests/testthat/test-python-arrays.R +++ b/tests/testthat/test-python-arrays.R @@ -76,6 +76,8 @@ def apply_mask(x, mask): py_logical_array <- r_to_py(r_logical_array) py_index_array <- r_to_py(r_index_array) + if (is_windows()) # not sure why ints cast to doubles on windows + storage.mode(r_index_array) <- "double" # check that round-triping gives an identical array expect_identical(r_logical_array, py_to_r(py_logical_array)) expect_identical(r_index_array, py_to_r(py_index_array)) From 67b77ccef930551195a71e3924943ec41fd72d0e Mon Sep 17 00:00:00 2001 From: Tomasz Kalinowski Date: Sat, 22 Feb 2025 14:20:08 -0500 Subject: [PATCH 6/7] more test fixes --- tests/testthat/test-interrupts.R | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/testthat/test-interrupts.R b/tests/testthat/test-interrupts.R index 0f50e9c6..39b7bfcd 100644 --- a/tests/testthat/test-interrupts.R +++ b/tests/testthat/test-interrupts.R @@ -8,7 +8,8 @@ test_that("Running Python scripts can be interrupted", { # interrupt this process shortly interruptor <- callr::r_bg(args = list(pid = Sys.getpid()), function(pid) { Sys.sleep(1) - ps::ps_interrupt(ps::ps_handle(pid)) + system2("kill", c("-s", "INT", pid)) + # ps::ps_interrupt(ps::ps_handle(pid)) }) # tell Python to sleep @@ -189,7 +190,7 @@ test_that("interrupts can be caught by Python while calling R", { expect_identical(p$get_exit_status(), 0L) expect_identical( - p$read_output(), + p$read_all_output(), "Caught interrupt; Running finally; Python finished; R Finished!") }) From 240076d53a16a03b9e6baa62b0270eb064e62322 Mon Sep 17 00:00:00 2001 From: Tomasz Kalinowski Date: Mon, 24 Feb 2025 07:46:30 -0500 Subject: [PATCH 7/7] doc tweaks --- R/py_require.R | 19 ++++++++++--------- man/py_require.Rd | 18 ++++++++++-------- 2 files changed, 20 insertions(+), 17 deletions(-) diff --git a/R/py_require.R b/R/py_require.R index 2201d135..ff3daf2d 100644 --- a/R/py_require.R +++ b/R/py_require.R @@ -33,9 +33,9 @@ #' dependencies. Many `uv` options can be customized via environment #' variables, as described #' [here](https://docs.astral.sh/uv/configuration/environment/). For example: -#' - If temporarily offline, set `Sys.setenv(UV_OFFLINE=1)`. +#' - If temporarily offline, set `Sys.setenv(UV_OFFLINE = "1")`. #' - To use a different index: `Sys.setenv(UV_INDEX = "https://download.pytorch.org/whl/cpu")`. -#' - To allow resolving a prerelease dependency: `Sys.setenv(UV_PRERELEASE="allow")`. +#' - To allow resolving a prerelease dependency: `Sys.setenv(UV_PRERELEASE = "allow")`. #' #' ## Installing from alternate sources #' @@ -98,18 +98,19 @@ #' @param action Determines how `py_require()` processes the provided #' requirements. Options are: #' - `add`: Adds the entries to the current set of requirements. -#' - `remove`: Removes _exact_ matches from the requirements list. For example, -#' if `"numpy==2.2.2"` is in the list, passing `"numpy"` with `action = -#' "remove"` will not remove it. Requests to remove nonexistent entries are -#' ignored. +#' - `remove`: Removes _exact_ matches from the requirements list. Requests to remove nonexistent entries are +#' ignored. For example, if `"numpy==2.2.2"` is in the list, passing `"numpy"` +#' with `action = "remove"` will not remove it. #' - `set`: Clears all existing requirements and replaces them with the #' provided ones. Packages and the Python version can be set independently. #' -#' @param exclude_newer Restricts package versions to those published before a +#' @param exclude_newer Limit package versions to those published before a #' specified date. This offers a lightweight alternative to freezing package #' versions, helping guard against Python package updates that break a -#' workflow. Once `exclude_newer` is set, only the `set` action can override -#' it. +#' workflow. Accepts strings formatted as RFC 3339 timestamps (e.g., +#' "2006-12-02T02:07:43Z") and local dates in the same format (e.g., +#' "2006-12-02") in your system's configured time zone. Once `exclude_newer` +#' is set, only the `set` action can override it. #' #' @returns `py_require()` is primarily called for its side effect of modifying #' the manifest of "Python requirements" for the current R session that diff --git a/man/py_require.Rd b/man/py_require.Rd index 36f387fb..8f85eb63 100644 --- a/man/py_require.Rd +++ b/man/py_require.Rd @@ -23,19 +23,21 @@ from local files or a git repository is also supported (see details).} \item{...}{Reserved for future extensions; must be empty.} -\item{exclude_newer}{Restricts package versions to those published before a +\item{exclude_newer}{Limit package versions to those published before a specified date. This offers a lightweight alternative to freezing package versions, helping guard against Python package updates that break a -workflow. Once \code{exclude_newer} is set, only the \code{set} action can override -it.} +workflow. Accepts strings formatted as RFC 3339 timestamps (e.g., +"2006-12-02T02:07:43Z") and local dates in the same format (e.g., +"2006-12-02") in your system's configured time zone. Once \code{exclude_newer} +is set, only the \code{set} action can override it.} \item{action}{Determines how \code{py_require()} processes the provided requirements. Options are: \itemize{ \item \code{add}: Adds the entries to the current set of requirements. -\item \code{remove}: Removes \emph{exact} matches from the requirements list. For example, -if \code{"numpy==2.2.2"} is in the list, passing \code{"numpy"} with \code{action = "remove"} will not remove it. Requests to remove nonexistent entries are -ignored. +\item \code{remove}: Removes \emph{exact} matches from the requirements list. Requests to remove nonexistent entries are +ignored. For example, if \code{"numpy==2.2.2"} is in the list, passing \code{"numpy"} +with \code{action = "remove"} will not remove it. \item \code{set}: Clears all existing requirements and replaces them with the provided ones. Packages and the Python version can be set independently. }} @@ -85,9 +87,9 @@ dependencies. Many \code{uv} options can be customized via environment variables, as described \href{https://docs.astral.sh/uv/configuration/environment/}{here}. For example: \itemize{ -\item If temporarily offline, set \code{Sys.setenv(UV_OFFLINE=1)}. +\item If temporarily offline, set \code{Sys.setenv(UV_OFFLINE = "1")}. \item To use a different index: \code{Sys.setenv(UV_INDEX = "https://download.pytorch.org/whl/cpu")}. -\item To allow resolving a prerelease dependency: \code{Sys.setenv(UV_PRERELEASE="allow")}. +\item To allow resolving a prerelease dependency: \code{Sys.setenv(UV_PRERELEASE = "allow")}. } \subsection{Installing from alternate sources}{