From 466f93503db0fd1a381733a2ced343f72bee4f0c Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 16 Aug 2026 13:37:35 +0000 Subject: [PATCH 1/4] fix: call a function passed as an argument from the caller's environment igraph functions that take another function as an argument call it themselves. lifecycle attributes a deprecation to the caller of the deprecated function, and that caller is an igraph frame: `deprecate_soft()` says nothing at all, and `deprecate_warn()` blames igraph and asks the user to report a bug against it. Either way `plot(g, layout = layout.circle)` never tells the user that the layout function they chose is on its way out. `bfs()`, `dfs()`, `arpack()` and `cluster_leading_eigen()` already evaluate their callback in the environment they were called from, through their `rho` and `env` arguments, and are attributed correctly because of it. `call_user_callback()` and `as_user_callback()` extend that to the remaining function arguments, without an argument to pass in and thread through: the function is called from a closure enclosed in the environment igraph was called from, which is the one lifecycle asks about. Nothing is inspected and nothing is replayed, so this covers any deprecation the function may signal, including one that depends on how it was called. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01WJx1uH2GvN5DcqdVzDLvwW --- R/attributes.R | 6 + R/cliques.R | 2 + R/cycles.R | 1 + R/layout.R | 3 +- R/motifs.R | 1 + R/plot.common.R | 2 +- R/plot.shapes.R | 5 + R/printr.R | 5 +- R/scan.R | 1 + R/tkplot.R | 1 + R/topology.R | 2 + R/utils-user-callbacks.R | 62 +++++++ tests/testthat/test-utils-user-callbacks.R | 196 +++++++++++++++++++++ 13 files changed, 283 insertions(+), 4 deletions(-) create mode 100644 R/utils-user-callbacks.R create mode 100644 tests/testthat/test-utils-user-callbacks.R diff --git a/R/attributes.R b/R/attributes.R index 13824155c1c..a801b92f1db 100644 --- a/R/attributes.R +++ b/R/attributes.R @@ -1327,6 +1327,12 @@ igraph.i.attribute.combination <- function(comb, allow_rename = FALSE) { "Attribute combination element must be a function or character scalar." ) } + # A plain loop, not lapply(): as_user_callback() reads the call stack to + # find out who igraph was called by, and a functional in between would hide + # the caller behind a frame of its own. + for (i in seq_along(comb)) { + comb[[i]] <- as_user_callback(comb[[i]]) + } if (is.null(names(comb))) { names(comb) <- rep("", length(comb)) } diff --git a/R/cliques.R b/R/cliques.R index 29fefbe4a65..d6e14c67acd 100644 --- a/R/cliques.R +++ b/R/cliques.R @@ -257,6 +257,7 @@ clique.number <- function(graph) { cliques <- function(graph, min = NULL, max = NULL, ..., callback = NULL) { ensure_igraph(graph) check_dots_empty() + callback <- as_user_callback(callback) if (is.null(callback)) { # Collector mode: use original implementation @@ -308,6 +309,7 @@ max_cliques <- function( ) { ensure_igraph(graph) check_dots_empty() + callback <- as_user_callback(callback) # Handle file and subset modes (original functionality) if (!is.null(file)) { diff --git a/R/cycles.R b/R/cycles.R index 6859cdf1f3f..bd912fd506e 100644 --- a/R/cycles.R +++ b/R/cycles.R @@ -166,6 +166,7 @@ simple_cycles <- function( # Argument checks ensure_igraph(graph) check_dots_empty() + callback <- as_user_callback(callback) if (is.null(callback)) { simple_cycles_impl( diff --git a/R/layout.R b/R/layout.R index 5c028fe8959..4798fd97a57 100644 --- a/R/layout.R +++ b/R/layout.R @@ -1181,7 +1181,7 @@ layout_nicely <- function(graph, dim = 2, ...) { lay <- graph_attr(graph, "layout") if (is.function(lay)) { if (!identical(lay, layout_nicely)) { - return(lay(graph, ...)) + return(call_user_callback(lay, graph, ...)) } else { # nop, we'll deal with it later below } @@ -3409,6 +3409,7 @@ layout_components <- function(graph, layout = NULL, ...) { if (is.null(layout)) { layout <- layout_with_kk } + layout <- as_user_callback(layout) V(graph)$id <- seq(vcount(graph)) gl <- decompose(graph) diff --git a/R/motifs.R b/R/motifs.R index 97a526b2905..10554081d49 100644 --- a/R/motifs.R +++ b/R/motifs.R @@ -221,6 +221,7 @@ motifs <- function( if (!is.null(cut.prob) && length(cut.prob) != size) { cli::cli_abort("{.arg cut.prob} must be the same length as {.arg size}") } + callback <- as_user_callback(callback) # If callback is provided, use the callback implementation if (!is.null(callback)) { diff --git a/R/plot.common.R b/R/plot.common.R index 9fb2c5783f6..1a7befc07c9 100644 --- a/R/plot.common.R +++ b/R/plot.common.R @@ -552,7 +552,7 @@ i.parse.plot.params <- function(graph, params) { ret <- function() { v <- p[[type]][[name]] if (is.function(v) && !dontcall) { - v <- v(graph) + v <- call_user_callback(v, graph) } if (is.null(range)) { return(v) diff --git a/R/plot.shapes.R b/R/plot.shapes.R index dc1eb672675..505a9ef8e62 100644 --- a/R/plot.shapes.R +++ b/R/plot.shapes.R @@ -459,6 +459,11 @@ add_shape <- function( )) } + # Wrapped here, where the user chose them, rather than at the plot call + # that eventually reaches them. + clip <- as_user_callback(clip) + plot <- as_user_callback(plot) + assign(shape, value = list(clip = clip, plot = plot), envir = .igraph.shapes) do.call(igraph_options, parameters) invisible(TRUE) diff --git a/R/printr.R b/R/printr.R index 7f5294859a8..065ade566d9 100644 --- a/R/printr.R +++ b/R/printr.R @@ -35,7 +35,7 @@ printer_callback <- function(fun) { if (!is.function(fun)) { warning("'fun' is not a function") } - add_class(fun, "printer_callback") + add_class(as_user_callback(fun), "printer_callback") } #' Is this a printer callback? @@ -58,7 +58,7 @@ print_footer <- function(footer) { } print_head_foot <- function(head_foot) { - if (is.function(head_foot)) head_foot() else cat(head_foot) + if (is.function(head_foot)) call_user_callback(head_foot) else cat(head_foot) } #' Print the only the head of an R object @@ -199,6 +199,7 @@ indent_print <- function(..., .indent = " ", .printer = NULL) { if (is.null(.printer)) { .printer <- print } + .printer <- as_user_callback(.printer) if (length(.indent) != 1 || !is.character(.indent)) { indent <- .indent # cli literal cannot start with a dot diff --git a/R/scan.R b/R/scan.R index a719650c8d1..c37304c84b9 100644 --- a/R/scan.R +++ b/R/scan.R @@ -130,6 +130,7 @@ local_scan <- function( stopifnot( is.null(FUN) || is.function(FUN) || (is.character(FUN) && length(FUN) == 1) ) + FUN <- as_user_callback(FUN) ## Logical stopifnot(is.logical(weighted), length(weighted) == 1) diff --git a/R/tkplot.R b/R/tkplot.R index 9342a284939..e956d7996d9 100644 --- a/R/tkplot.R +++ b/R/tkplot.R @@ -700,6 +700,7 @@ tk_center <- function(tkp.id) { #' @export tk_reshape <- function(tkp.id, newlayout, ..., params) { # nocov start + newlayout <- as_user_callback(newlayout) tkp <- .tkplot.get(tkp.id) new_coords <- do_call( newlayout, diff --git a/R/topology.R b/R/topology.R index 1069c076de5..3994cea7ead 100644 --- a/R/topology.R +++ b/R/topology.R @@ -1054,6 +1054,7 @@ graph.count.subisomorphisms.vf2 <- function( #' @family graph isomorphism isomorphisms <- function(graph1, graph2, method = "vf2", ..., callback = NULL) { method <- igraph_match_arg(method) + callback <- as_user_callback(callback) if (method != "vf2") { cli::cli_abort( @@ -1185,6 +1186,7 @@ subgraph_isomorphisms <- function( callback = NULL ) { method <- igraph_match_arg(method) + callback <- as_user_callback(callback) if (!is.null(callback) && method != "vf2") { cli::cli_abort( diff --git a/R/utils-user-callbacks.R b/R/utils-user-callbacks.R new file mode 100644 index 00000000000..9abd900d683 --- /dev/null +++ b/R/utils-user-callbacks.R @@ -0,0 +1,62 @@ +# Call a function the user passed as an argument, as if they had called it +# themselves. +# +# lifecycle attributes a deprecation to the caller of the deprecated function. +# When igraph calls a function the user handed it, that caller is an igraph +# frame: `deprecate_soft()` then says nothing at all, and `deprecate_warn()` +# blames igraph and asks the user to report a bug against it. Neither tells the +# user that the function they chose is on its way out, which is what they need +# to hear before the deprecation becomes hard. +# +# `bfs()`, `dfs()`, `arpack()` and `cluster_leading_eigen()` already evaluate +# their callback in the environment they were called from, through their `rho` +# and `env` arguments, and are attributed correctly because of it. These +# helpers extend that treatment to the remaining function arguments, without an +# argument to pass in and thread through. +call_user_callback <- function(fn, ...) { + as_user_callback(fn)(...) +} + +# `fn`, wrapped so that it is called from the environment igraph was called +# from. Anything but a function, `NULL` included, is returned unchanged. +# +# Use this for a function that igraph hands on -- to the C layer, or to a later +# call -- rather than calls itself; the environment is the one current when the +# function was passed, which is where the user chose it. +as_user_callback <- function(fn) { + if (!is.function(fn)) { + return(fn) + } + + # The wrapper is enclosed in a child of the user's environment, so that `fn` + # sees a caller that belongs to the user rather than to igraph. lifecycle + # asks `topenv()` who that caller is, and `topenv()` looks through the child. + rlang::new_function( + args = rlang::pairlist2(... = ), + body = quote(fn(...)), + env = rlang::env(igraph_user_env(), fn = fn) + ) +} + +# The environment igraph was called from: the innermost caller that does not +# belong to igraph itself. +# +# Walking the caller chain keeps this independent of how deeply a function +# argument is passed on inside igraph before it is called, which a fixed +# `rlang::caller_env(n)` would have to track. Callers must not reach this +# through a functional such as `lapply()`, whose frame would end the walk in +# place of the user's. +igraph_user_env <- function() { + ns <- topenv(environment(igraph_user_env)) + + generation <- 1L + repeat { + env <- parent.frame(generation) + # parent.frame() bottoms out at the global environment, so the walk + # terminates there even if every frame belongs to igraph. + if (identical(env, globalenv()) || !identical(topenv(env), ns)) { + return(env) + } + generation <- generation + 1L + } +} diff --git a/tests/testthat/test-utils-user-callbacks.R b/tests/testthat/test-utils-user-callbacks.R new file mode 100644 index 00000000000..5c07773f21b --- /dev/null +++ b/tests/testthat/test-utils-user-callbacks.R @@ -0,0 +1,196 @@ +# Call `fn(...)` the way a user would. +# +# Two things stand between a test and what a user sees. The environment a test +# runs in belongs to igraph, and lifecycle exempts the test suite of the +# deprecating package from its "is this the user's doing?" question, treating +# igraph's own frames as direct (see setup-lifecycle.R). Both would make these +# tests pass whether or not the callback is attributed to its caller. So call +# from an environment that belongs to no package, with the exemption cleared. +as_user <- function(fn, ...) { + withr::local_envvar(TESTTHAT_PKG = "") + user <- rlang::new_function( + args = rlang::pairlist2(... = ), + body = quote(fn(...)), + env = rlang::env(globalenv(), fn = fn) + ) + user(...) +} + +expect_deprecation <- function(expr, regexp) { + warnings <- testthat::capture_warnings(expr) + expect_gt(length(warnings), 0) + expect_match(warnings, regexp, all = TRUE) + # The point of the exercise: lifecycle appends this to a deprecation it holds + # igraph responsible for, and asks the user to report a bug about it. + expect_no_match(warnings, "likely used in") +} + +# A function that igraph deprecates, without the noise of one that also does +# something. Its name is what tells lifecycle's deduplication one deprecation +# apart from the other, so each test needs its own. +deprecated_igraph_function <- function( + what, + signaller = "deprecate_soft", + value = quote(invisible(NULL)) +) { + rlang::new_function( + args = rlang::pairlist2(... = ), + body = call( + "{", + rlang::call2(signaller, "2.0.0", what, .ns = "lifecycle"), + value + ), + env = asNamespace("igraph") + ) +} + +# An igraph function that hands a function argument on, with and without the +# treatment under test. +igraph_function <- function(wrapped) { + fn <- if (wrapped) { + function(callback, ...) call_user_callback(callback, ...) + } else { + function(callback, ...) callback(...) + } + environment(fn) <- asNamespace("igraph") + fn +} + +test_that("as_user_callback() leaves everything but a function alone", { + expect_null(as_user_callback(NULL)) + expect_identical(as_user_callback("ecount"), "ecount") +}) + +test_that("a callback is called by the user, not by igraph", { + caller_package <- function() environmentName(topenv(parent.frame())) + + expect_equal(as_user(igraph_function(FALSE), caller_package), "igraph") + expect_equal(as_user(igraph_function(TRUE), caller_package), "R_GlobalEnv") +}) + +test_that("a soft deprecation reaches the user that caused it", { + # Silent as things stand: lifecycle sees an igraph frame as the caller, and + # does not report a package's own use of a softly deprecated function. + expect_no_warning( + as_user(igraph_function(FALSE), deprecated_igraph_function("soft_silent()")) + ) + + expect_deprecation( + as_user(igraph_function(TRUE), deprecated_igraph_function("soft_heard()")), + "soft_heard" + ) +}) + +test_that("a soft deprecation stays silent for another package", { + fn <- deprecated_igraph_function("soft_other_package()") + call_igraph <- function() call_user_callback(fn) + environment(call_igraph) <- rlang::env(asNamespace("stats"), fn = fn) + igraph_caller <- function() call_igraph() + environment(igraph_caller) <- rlang::env( + asNamespace("igraph"), + call_igraph = call_igraph + ) + + withr::local_envvar(TESTTHAT_PKG = "") + expect_no_warning(igraph_caller()) +}) + +test_that("a warning deprecation stops blaming igraph for the user's choice", { + blamed <- testthat::capture_warnings( + as_user( + igraph_function(FALSE), + deprecated_igraph_function("warn_blamed()", "deprecate_warn") + ) + ) + expect_match(blamed, "likely used in the .*igraph.* package") + + attributed <- testthat::capture_warnings( + as_user( + igraph_function(TRUE), + deprecated_igraph_function("warn_attributed()", "deprecate_warn") + ) + ) + expect_no_match(attributed, "likely used in") +}) + +# ---- deprecated functions passed as arguments ------------------------- + +test_that("plot() reports a deprecated layout function", { + g <- make_ring(5) + withr::local_pdf(NULL) + + expect_deprecation( + as_user(plot, g, layout = layout.circle), + "layout_in_circle" + ) +}) + +test_that("plot() reports a deprecated layout graph attribute", { + g <- make_ring(5) + g$layout <- layout.random + withr::local_pdf(NULL) + + expect_deprecation(as_user(plot, g), "layout_randomly") +}) + +test_that("plot() is silent about a current layout function", { + g <- make_ring(5) + withr::local_pdf(NULL) + + expect_no_condition(plot(g, layout = layout_in_circle)) +}) + +test_that("layout_nicely() reports a deprecated layout graph attribute", { + g <- make_ring(5) + g$layout <- layout.circle + + expect_deprecation(as_user(layout_nicely, g), "layout_in_circle") +}) + +test_that("layout_components() reports a deprecated layout function", { + g <- make_ring(5) + make_ring(4) + + expect_deprecation( + as_user(layout_components, g, layout.circle), + "layout_in_circle" + ) +}) + +test_that("add_shape() reports a deprecated shape function when it is used", { + g <- make_ring(5) + withr::local_pdf(NULL) + + as_user(add_shape, "test-noplot", plot = igraph.shape.noplot) + expect_deprecation(plot(g, vertex.shape = "test-noplot"), "shape_noplot") +}) + +test_that("local_scan() reports a deprecated FUN", { + g <- make_ring(5) + + expect_deprecation( + as_user(local_scan, g, FUN = graph.density), + "edge_density" + ) +}) + +test_that("attribute combinations report a deprecated function", { + g <- make_graph(c(1, 2, 1, 2)) + E(g)$weight <- c(1, 2) + + # `is.igraph()` is not a meaningful combiner, but it is softly deprecated and + # accepts the vector of attribute values that a combiner is handed. + expect_deprecation( + as_user(simplify, g, edge.attr.comb = list(weight = is.igraph)), + "is_igraph" + ) +}) + +test_that("callbacks report a deprecated function", { + g <- make_ring(3) + + # Called with each clique, and FALSE keeps the search going. + expect_deprecation( + as_user(cliques, g, min = 2, callback = is.igraph), + "is_igraph" + ) +}) From 7e6a3be68eb4c6689bf33d4b03789ef1e7795186 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 16 Aug 2026 13:37:53 +0000 Subject: [PATCH 2/4] fix: un-deprecate the layout functions that are passed to `plot()` `layout.spring()`, `layout.svd()` and `layout.fruchterman.reingold.grid()` are deprecated with a warning again, and lay out with `layout_with_fr()` as before. They are layout callbacks, and their hard deprecation -- made after 2.3.3 and never released -- would break `plot(layout = )` for users who have never seen the warning, because igraph, not they, called the function. Their documentation already describes this behaviour. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01WJx1uH2GvN5DcqdVzDLvwW --- NEWS.md | 3 --- R/layout.R | 9 ++++++--- tests/testthat/test-utils-user-callbacks.R | 18 ++++++++++++++++++ 3 files changed, 24 insertions(+), 6 deletions(-) diff --git a/NEWS.md b/NEWS.md index a24bf34615d..3a9a942edeb 100644 --- a/NEWS.md +++ b/NEWS.md @@ -899,9 +899,6 @@ - `layout.kamada.kawai()` is now deprecated with a warning instead of a message. - `layout.lgl()` is now deprecated with a warning instead of a message. - `layout_with_mds()` is now deprecated with a warning instead of a message. -- `layout.spring()` is now defunct (errors instead of warning). -- `layout.svd()` is now defunct (errors instead of warning). -- `layout.fruchterman.reingold.grid()` is now defunct (errors instead of warning). - `layout.drl()` is now deprecated with a warning instead of a message. - `nei()` has been removed. - `innei()` has been removed. diff --git a/R/layout.R b/R/layout.R index 4798fd97a57..f3eae20369f 100644 --- a/R/layout.R +++ b/R/layout.R @@ -3434,7 +3434,8 @@ layout_components <- function(graph, layout = NULL, ...) { #' @export #' @keywords internal layout.spring <- function(graph, ...) { - lifecycle::deprecate_stop("2.1.0", "layout.spring()", "layout_with_fr()") + lifecycle::deprecate_warn("2.1.0", "layout.spring()", "layout_with_fr()") + layout_with_fr(graph) } #' SVD layout, this was removed from igraph @@ -3451,7 +3452,8 @@ layout.spring <- function(graph, ...) { #' @keywords internal #' @export layout.svd <- function(graph, ...) { - lifecycle::deprecate_stop("2.1.0", "layout.svd()", "layout_with_fr()") + lifecycle::deprecate_warn("2.1.0", "layout.svd()", "layout_with_fr()") + layout_with_fr(graph) } #' Grid Fruchterman-Reingold layout, this was removed from igraph @@ -3469,11 +3471,12 @@ layout.svd <- function(graph, ...) { #' @keywords internal #' @export layout.fruchterman.reingold.grid <- function(graph, ...) { - lifecycle::deprecate_stop( + lifecycle::deprecate_warn( "2.1.0", "layout.fruchterman.reingold.grid()", "layout_with_fr()" ) + layout_with_fr(graph) } #' The DrL graph layout generator diff --git a/tests/testthat/test-utils-user-callbacks.R b/tests/testthat/test-utils-user-callbacks.R index 5c07773f21b..e3160b4beef 100644 --- a/tests/testthat/test-utils-user-callbacks.R +++ b/tests/testthat/test-utils-user-callbacks.R @@ -194,3 +194,21 @@ test_that("callbacks report a deprecated function", { "is_igraph" ) }) + +# ---- the layout functions plot() may be handed ------------------------- + +test_that("the layout functions that plot() may be handed still work", { + g <- make_ring(5) + + expect_deprecation(coords <- layout.spring(g), "layout_with_fr") + expect_equal(dim(coords), c(5, 2)) + + expect_deprecation(coords <- layout.svd(g), "layout_with_fr") + expect_equal(dim(coords), c(5, 2)) + + expect_deprecation( + coords <- layout.fruchterman.reingold.grid(g), + "layout_with_fr" + ) + expect_equal(dim(coords), c(5, 2)) +}) From b9e9b4581c7f4846bf490bd47c3c7b988da17702 Mon Sep 17 00:00:00 2001 From: krlmlr Date: Sun, 16 Aug 2026 13:46:57 +0000 Subject: [PATCH 3/4] chore: Auto-update from GitHub Actions Run: https://github.com/igraph/rigraph/actions/runs/31950457376 --- man/layout.fruchterman.reingold.grid.Rd | 4 ++++ man/layout.spring.Rd | 4 ++++ man/layout.svd.Rd | 4 ++++ 3 files changed, 12 insertions(+) diff --git a/man/layout.fruchterman.reingold.grid.Rd b/man/layout.fruchterman.reingold.grid.Rd index 67368c0ed9a..c9bd2d65c89 100644 --- a/man/layout.fruchterman.reingold.grid.Rd +++ b/man/layout.fruchterman.reingold.grid.Rd @@ -19,4 +19,8 @@ Layout coordinates, a two column matrix. Now it calls the Fruchterman-Reingold layout \code{\link[=layout_with_fr]{layout_with_fr()}}. } +\section{Related documentation in the C library}{ +\href{https://igraph.org/c/html/0.10.17/igraph-Basic.html#igraph_vcount}{\code{vcount()}}, \href{https://igraph.org/c/html/0.10.17/igraph-Basic.html#igraph_edges}{\code{edges()}}, \href{https://igraph.org/c/html/0.10.17/igraph-Basic.html#igraph_get_eids}{\code{get_eids()}}, \href{https://igraph.org/c/html/0.10.17/igraph-Basic.html#igraph_ecount}{\code{ecount()}} +} + \keyword{internal} diff --git a/man/layout.spring.Rd b/man/layout.spring.Rd index b87db5942d6..b876cffadcd 100644 --- a/man/layout.spring.Rd +++ b/man/layout.spring.Rd @@ -19,4 +19,8 @@ Layout coordinates, a two column matrix. Now it calls the Fruchterman-Reingold layout \code{\link[=layout_with_fr]{layout_with_fr()}}. } +\section{Related documentation in the C library}{ +\href{https://igraph.org/c/html/0.10.17/igraph-Basic.html#igraph_vcount}{\code{vcount()}}, \href{https://igraph.org/c/html/0.10.17/igraph-Basic.html#igraph_edges}{\code{edges()}}, \href{https://igraph.org/c/html/0.10.17/igraph-Basic.html#igraph_get_eids}{\code{get_eids()}}, \href{https://igraph.org/c/html/0.10.17/igraph-Basic.html#igraph_ecount}{\code{ecount()}} +} + \keyword{internal} diff --git a/man/layout.svd.Rd b/man/layout.svd.Rd index c83ee8cc0a7..08f44ed6d11 100644 --- a/man/layout.svd.Rd +++ b/man/layout.svd.Rd @@ -19,4 +19,8 @@ Layout coordinates, a two column matrix. Now it calls the Fruchterman-Reingold layout \code{\link[=layout_with_fr]{layout_with_fr()}}. } +\section{Related documentation in the C library}{ +\href{https://igraph.org/c/html/0.10.17/igraph-Basic.html#igraph_vcount}{\code{vcount()}}, \href{https://igraph.org/c/html/0.10.17/igraph-Basic.html#igraph_edges}{\code{edges()}}, \href{https://igraph.org/c/html/0.10.17/igraph-Basic.html#igraph_get_eids}{\code{get_eids()}}, \href{https://igraph.org/c/html/0.10.17/igraph-Basic.html#igraph_ecount}{\code{ecount()}} +} + \keyword{internal} From 3bb7d95eb440097c3cdc3f27385c7bf27d35b5d6 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 16 Aug 2026 14:04:51 +0000 Subject: [PATCH 4/4] Revert the un-deprecation, and snapshot the messages The hard deprecation of `layout.spring()`, `layout.svd()` and `layout.fruchterman.reingold.grid()` stands: a defunct layout function passed to `plot()` now fails against the call that chose it, which is what un-deprecating them was for. This reverts commit 7e6a3be68eb4c6689bf33d4b03789ef1e7795186 and the documentation the roxygen run derived from it. The tests that assert on message output are snapshot tests now. One of them records what a deprecation looked like before and after in the same place, which says more than a regular expression can. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01WJx1uH2GvN5DcqdVzDLvwW --- NEWS.md | 3 + R/layout.R | 9 +- man/layout.fruchterman.reingold.grid.Rd | 4 - man/layout.spring.Rd | 4 - man/layout.svd.Rd | 4 - tests/testthat/_snaps/utils-user-callbacks.md | 98 ++++++++++++++ tests/testthat/test-utils-user-callbacks.R | 127 +++++++----------- 7 files changed, 153 insertions(+), 96 deletions(-) create mode 100644 tests/testthat/_snaps/utils-user-callbacks.md diff --git a/NEWS.md b/NEWS.md index 3a9a942edeb..a24bf34615d 100644 --- a/NEWS.md +++ b/NEWS.md @@ -899,6 +899,9 @@ - `layout.kamada.kawai()` is now deprecated with a warning instead of a message. - `layout.lgl()` is now deprecated with a warning instead of a message. - `layout_with_mds()` is now deprecated with a warning instead of a message. +- `layout.spring()` is now defunct (errors instead of warning). +- `layout.svd()` is now defunct (errors instead of warning). +- `layout.fruchterman.reingold.grid()` is now defunct (errors instead of warning). - `layout.drl()` is now deprecated with a warning instead of a message. - `nei()` has been removed. - `innei()` has been removed. diff --git a/R/layout.R b/R/layout.R index f3eae20369f..4798fd97a57 100644 --- a/R/layout.R +++ b/R/layout.R @@ -3434,8 +3434,7 @@ layout_components <- function(graph, layout = NULL, ...) { #' @export #' @keywords internal layout.spring <- function(graph, ...) { - lifecycle::deprecate_warn("2.1.0", "layout.spring()", "layout_with_fr()") - layout_with_fr(graph) + lifecycle::deprecate_stop("2.1.0", "layout.spring()", "layout_with_fr()") } #' SVD layout, this was removed from igraph @@ -3452,8 +3451,7 @@ layout.spring <- function(graph, ...) { #' @keywords internal #' @export layout.svd <- function(graph, ...) { - lifecycle::deprecate_warn("2.1.0", "layout.svd()", "layout_with_fr()") - layout_with_fr(graph) + lifecycle::deprecate_stop("2.1.0", "layout.svd()", "layout_with_fr()") } #' Grid Fruchterman-Reingold layout, this was removed from igraph @@ -3471,12 +3469,11 @@ layout.svd <- function(graph, ...) { #' @keywords internal #' @export layout.fruchterman.reingold.grid <- function(graph, ...) { - lifecycle::deprecate_warn( + lifecycle::deprecate_stop( "2.1.0", "layout.fruchterman.reingold.grid()", "layout_with_fr()" ) - layout_with_fr(graph) } #' The DrL graph layout generator diff --git a/man/layout.fruchterman.reingold.grid.Rd b/man/layout.fruchterman.reingold.grid.Rd index c9bd2d65c89..67368c0ed9a 100644 --- a/man/layout.fruchterman.reingold.grid.Rd +++ b/man/layout.fruchterman.reingold.grid.Rd @@ -19,8 +19,4 @@ Layout coordinates, a two column matrix. Now it calls the Fruchterman-Reingold layout \code{\link[=layout_with_fr]{layout_with_fr()}}. } -\section{Related documentation in the C library}{ -\href{https://igraph.org/c/html/0.10.17/igraph-Basic.html#igraph_vcount}{\code{vcount()}}, \href{https://igraph.org/c/html/0.10.17/igraph-Basic.html#igraph_edges}{\code{edges()}}, \href{https://igraph.org/c/html/0.10.17/igraph-Basic.html#igraph_get_eids}{\code{get_eids()}}, \href{https://igraph.org/c/html/0.10.17/igraph-Basic.html#igraph_ecount}{\code{ecount()}} -} - \keyword{internal} diff --git a/man/layout.spring.Rd b/man/layout.spring.Rd index b876cffadcd..b87db5942d6 100644 --- a/man/layout.spring.Rd +++ b/man/layout.spring.Rd @@ -19,8 +19,4 @@ Layout coordinates, a two column matrix. Now it calls the Fruchterman-Reingold layout \code{\link[=layout_with_fr]{layout_with_fr()}}. } -\section{Related documentation in the C library}{ -\href{https://igraph.org/c/html/0.10.17/igraph-Basic.html#igraph_vcount}{\code{vcount()}}, \href{https://igraph.org/c/html/0.10.17/igraph-Basic.html#igraph_edges}{\code{edges()}}, \href{https://igraph.org/c/html/0.10.17/igraph-Basic.html#igraph_get_eids}{\code{get_eids()}}, \href{https://igraph.org/c/html/0.10.17/igraph-Basic.html#igraph_ecount}{\code{ecount()}} -} - \keyword{internal} diff --git a/man/layout.svd.Rd b/man/layout.svd.Rd index 08f44ed6d11..c83ee8cc0a7 100644 --- a/man/layout.svd.Rd +++ b/man/layout.svd.Rd @@ -19,8 +19,4 @@ Layout coordinates, a two column matrix. Now it calls the Fruchterman-Reingold layout \code{\link[=layout_with_fr]{layout_with_fr()}}. } -\section{Related documentation in the C library}{ -\href{https://igraph.org/c/html/0.10.17/igraph-Basic.html#igraph_vcount}{\code{vcount()}}, \href{https://igraph.org/c/html/0.10.17/igraph-Basic.html#igraph_edges}{\code{edges()}}, \href{https://igraph.org/c/html/0.10.17/igraph-Basic.html#igraph_get_eids}{\code{get_eids()}}, \href{https://igraph.org/c/html/0.10.17/igraph-Basic.html#igraph_ecount}{\code{ecount()}} -} - \keyword{internal} diff --git a/tests/testthat/_snaps/utils-user-callbacks.md b/tests/testthat/_snaps/utils-user-callbacks.md new file mode 100644 index 00000000000..e45e2305df6 --- /dev/null +++ b/tests/testthat/_snaps/utils-user-callbacks.md @@ -0,0 +1,98 @@ +# lifecycle names the user rather than igraph + + Code + as_user(igraph_function(FALSE), deprecated_igraph_function("soft_plain()")) + as_user(igraph_function(FALSE), deprecated_igraph_function("warn_plain()", + "deprecate_warn")) + Condition + Warning: + `warn_plain()` was deprecated in igraph 2.0.0. + i The deprecated feature was likely used in the igraph package. + Please report the issue at . + Code + as_user(igraph_function(TRUE), deprecated_igraph_function("soft_wrapped()")) + Condition + Warning: + `soft_wrapped()` was deprecated in igraph 2.0.0. + Code + as_user(igraph_function(TRUE), deprecated_igraph_function("warn_wrapped()", + "deprecate_warn")) + Condition + Warning: + `warn_wrapped()` was deprecated in igraph 2.0.0. + +# plot() reports a deprecated layout function + + Code + cat(warnings, sep = "\n") + Output + `layout.circle()` was deprecated in igraph 2.1.0. + i Please use `layout_in_circle()` instead. + +# plot() reports a deprecated layout graph attribute + + Code + cat(warnings, sep = "\n") + Output + `layout.random()` was deprecated in igraph 2.1.0. + i Please use `layout_randomly()` instead. + +# layout_nicely() reports a deprecated layout graph attribute + + Code + coords <- as_user(layout_nicely, g) + Condition + Warning: + `layout.circle()` was deprecated in igraph 2.1.0. + i Please use `layout_in_circle()` instead. + +# layout_components() reports a deprecated layout function + + Code + coords <- as_user(layout_components, g, layout.circle) + Condition + Warning: + `layout.circle()` was deprecated in igraph 2.1.0. + i Please use `layout_in_circle()` instead. + Warning: + `layout.circle()` was deprecated in igraph 2.1.0. + i Please use `layout_in_circle()` instead. + +# add_shape() reports a deprecated shape function when it is used + + Code + cat(warnings, sep = "\n") + Output + `igraph.shape.noplot()` was deprecated in igraph 2.0.0. + i Please use `shape_noplot()` instead. + +# local_scan() reports a deprecated FUN + + Code + scan <- as_user(local_scan, g, FUN = graph.density) + Condition + Warning: + `graph.density()` was deprecated in igraph 2.0.0. + i Please use `edge_density()` instead. + Warning: + `graph.density()` was deprecated in igraph 2.0.0. + i Please use `edge_density()` instead. + +# attribute combinations report a deprecated function + + Code + simple <- as_user(simplify, g, edge.attr.comb = list(weight = is.igraph)) + Condition + Warning: + `is.igraph()` was deprecated in igraph 2.0.0. + i Please use `is_igraph()` instead. + +# callbacks report a deprecated function + + Code + as_user(cliques, g, min = 3, callback = is.igraph) + Condition + Warning: + `is.igraph()` was deprecated in igraph 2.0.0. + i Please use `is_igraph()` instead. + diff --git a/tests/testthat/test-utils-user-callbacks.R b/tests/testthat/test-utils-user-callbacks.R index e3160b4beef..3529973d664 100644 --- a/tests/testthat/test-utils-user-callbacks.R +++ b/tests/testthat/test-utils-user-callbacks.R @@ -16,15 +16,6 @@ as_user <- function(fn, ...) { user(...) } -expect_deprecation <- function(expr, regexp) { - warnings <- testthat::capture_warnings(expr) - expect_gt(length(warnings), 0) - expect_match(warnings, regexp, all = TRUE) - # The point of the exercise: lifecycle appends this to a deprecation it holds - # igraph responsible for, and asks the user to report a bug about it. - expect_no_match(warnings, "likely used in") -} - # A function that igraph deprecates, without the noise of one that also does # something. Its name is what tells lifecycle's deduplication one deprecation # apart from the other, so each test needs its own. @@ -68,17 +59,27 @@ test_that("a callback is called by the user, not by igraph", { expect_equal(as_user(igraph_function(TRUE), caller_package), "R_GlobalEnv") }) -test_that("a soft deprecation reaches the user that caused it", { - # Silent as things stand: lifecycle sees an igraph frame as the caller, and - # does not report a package's own use of a softly deprecated function. - expect_no_warning( - as_user(igraph_function(FALSE), deprecated_igraph_function("soft_silent()")) - ) +test_that("lifecycle names the user rather than igraph", { + rlang::local_options(lifecycle_verbosity = "warning") - expect_deprecation( - as_user(igraph_function(TRUE), deprecated_igraph_function("soft_heard()")), - "soft_heard" - ) + expect_snapshot({ + # As things stand, a soft deprecation goes unmentioned, because lifecycle + # does not report a package's own use of one ... + as_user(igraph_function(FALSE), deprecated_igraph_function("soft_plain()")) + + # ... and a warning deprecation asks the user to report an igraph bug. + as_user( + igraph_function(FALSE), + deprecated_igraph_function("warn_plain()", "deprecate_warn") + ) + + # Called from the environment igraph was called from, both name the user. + as_user(igraph_function(TRUE), deprecated_igraph_function("soft_wrapped()")) + as_user( + igraph_function(TRUE), + deprecated_igraph_function("warn_wrapped()", "deprecate_warn") + ) + }) }) test_that("a soft deprecation stays silent for another package", { @@ -95,42 +96,29 @@ test_that("a soft deprecation stays silent for another package", { expect_no_warning(igraph_caller()) }) -test_that("a warning deprecation stops blaming igraph for the user's choice", { - blamed <- testthat::capture_warnings( - as_user( - igraph_function(FALSE), - deprecated_igraph_function("warn_blamed()", "deprecate_warn") - ) - ) - expect_match(blamed, "likely used in the .*igraph.* package") - - attributed <- testthat::capture_warnings( - as_user( - igraph_function(TRUE), - deprecated_igraph_function("warn_attributed()", "deprecate_warn") - ) - ) - expect_no_match(attributed, "likely used in") -}) - # ---- deprecated functions passed as arguments ------------------------- test_that("plot() reports a deprecated layout function", { g <- make_ring(5) withr::local_pdf(NULL) + rlang::local_options(lifecycle_verbosity = "warning") - expect_deprecation( - as_user(plot, g, layout = layout.circle), - "layout_in_circle" + # expect_snapshot() has no way to replay the plot a plotting call records, + # so capture the warnings first and snapshot those. + warnings <- testthat::capture_warnings( + as_user(plot, g, layout = layout.circle) ) + expect_snapshot(cat(warnings, sep = "\n")) }) test_that("plot() reports a deprecated layout graph attribute", { g <- make_ring(5) g$layout <- layout.random withr::local_pdf(NULL) + rlang::local_options(lifecycle_verbosity = "warning") - expect_deprecation(as_user(plot, g), "layout_randomly") + warnings <- testthat::capture_warnings(as_user(plot, g)) + expect_snapshot(cat(warnings, sep = "\n")) }) test_that("plot() is silent about a current layout function", { @@ -143,72 +131,55 @@ test_that("plot() is silent about a current layout function", { test_that("layout_nicely() reports a deprecated layout graph attribute", { g <- make_ring(5) g$layout <- layout.circle + rlang::local_options(lifecycle_verbosity = "warning") - expect_deprecation(as_user(layout_nicely, g), "layout_in_circle") + expect_snapshot(coords <- as_user(layout_nicely, g)) }) test_that("layout_components() reports a deprecated layout function", { g <- make_ring(5) + make_ring(4) + rlang::local_options(lifecycle_verbosity = "warning") - expect_deprecation( - as_user(layout_components, g, layout.circle), - "layout_in_circle" - ) + expect_snapshot(coords <- as_user(layout_components, g, layout.circle)) }) test_that("add_shape() reports a deprecated shape function when it is used", { g <- make_ring(5) withr::local_pdf(NULL) + rlang::local_options(lifecycle_verbosity = "warning") as_user(add_shape, "test-noplot", plot = igraph.shape.noplot) - expect_deprecation(plot(g, vertex.shape = "test-noplot"), "shape_noplot") + warnings <- testthat::capture_warnings(plot(g, vertex.shape = "test-noplot")) + expect_snapshot(cat(warnings, sep = "\n")) }) test_that("local_scan() reports a deprecated FUN", { - g <- make_ring(5) + # Two vertices, so that the snapshot stays short: `FUN` is called for each of + # them, and the "warning" verbosity these tests need turns off the + # deduplication that leaves a user with one warning. + g <- make_ring(2) + rlang::local_options(lifecycle_verbosity = "warning") - expect_deprecation( - as_user(local_scan, g, FUN = graph.density), - "edge_density" - ) + expect_snapshot(scan <- as_user(local_scan, g, FUN = graph.density)) }) test_that("attribute combinations report a deprecated function", { g <- make_graph(c(1, 2, 1, 2)) E(g)$weight <- c(1, 2) + rlang::local_options(lifecycle_verbosity = "warning") # `is.igraph()` is not a meaningful combiner, but it is softly deprecated and # accepts the vector of attribute values that a combiner is handed. - expect_deprecation( - as_user(simplify, g, edge.attr.comb = list(weight = is.igraph)), - "is_igraph" + expect_snapshot( + simple <- as_user(simplify, g, edge.attr.comb = list(weight = is.igraph)) ) }) test_that("callbacks report a deprecated function", { g <- make_ring(3) + rlang::local_options(lifecycle_verbosity = "warning") - # Called with each clique, and FALSE keeps the search going. - expect_deprecation( - as_user(cliques, g, min = 2, callback = is.igraph), - "is_igraph" - ) -}) - -# ---- the layout functions plot() may be handed ------------------------- - -test_that("the layout functions that plot() may be handed still work", { - g <- make_ring(5) - - expect_deprecation(coords <- layout.spring(g), "layout_with_fr") - expect_equal(dim(coords), c(5, 2)) - - expect_deprecation(coords <- layout.svd(g), "layout_with_fr") - expect_equal(dim(coords), c(5, 2)) - - expect_deprecation( - coords <- layout.fruchterman.reingold.grid(g), - "layout_with_fr" - ) - expect_equal(dim(coords), c(5, 2)) + # Called with each clique -- the one triangle -- and FALSE keeps the search + # going. + expect_snapshot(as_user(cliques, g, min = 3, callback = is.igraph)) })