refactor: move optional arguments of trees functions behind the ellipsis

R/cycles.R

@@ -31,6 +31,7 @@
 #' a specific cycle.
 #'
 #' @param graph The input graph.
+#' @inheritParams rlang::args_dots_empty
 #' @param mode Character constant specifying how to handle directed graphs.
 #'   `out` follows edge directions, `in` follows edges in the reverse direction,
 #'   and `all` ignores edge directions. Ignored in undirected graphs.

R/decomposition.R

@@ -67,6 +67,7 @@ is.chordal <- function(
 #'
 #' @param graph The input graph. It may be directed, but edge directions are
 #'   ignored, as the algorithm is defined for undirected graphs.
+#' @inheritParams rlang::args_dots_empty
 #' @param alpha Numeric vector, the maximal chardinality ordering of the
 #'   vertices. If it is `NULL`, then it is automatically calculated by
 #'   calling [max_cardinality()], or from `alpham1` if
@@ -118,11 +119,44 @@ is.chordal <- function(
 #'
 is_chordal <- function(
   graph,
+  ...,
   alpha = NULL,
   alpham1 = NULL,
   fillin = FALSE,
   newgraph = FALSE
 ) {
+  # BEGIN GENERATED ARG_HANDLE: is_chordal, do not edit, see tools/generate-migrations.R
+  if (...length() > 0L) {
+    .arg_handle <- migrate_recover_args(
+      list(...),
+      current = list(
+        alpha = alpha,
+        alpham1 = alpham1,
+        fillin = fillin,
+        newgraph = newgraph
+      ),
+      recover_new = c("alpha", "alpham1", "fillin", "newgraph"),
+      recover_old = c("alpha", "alpham1", "fillin", "newgraph"),
+      match_names = c("alpha", "alpham1", "fillin", "newgraph"),
+      match_to = c("alpha", "alpham1", "fillin", "newgraph"),
+      defaults = list(
+        alpha = NULL,
+        alpham1 = NULL,
+        fillin = FALSE,
+        newgraph = FALSE
+      ),
+      head_args = c("graph"),
+      fn_name = "is_chordal"
+    )
+    list2env(.arg_handle$values, environment())
+    lifecycle::deprecate_soft(
+      "3.0.0",
+      what = I(.arg_handle$what),
+      details = .arg_handle$details
+    )
+  }
+  # END GENERATED ARG_HANDLE
+
   ensure_igraph(graph)
   if (!is.null(alpha)) {
     alpha <- as.numeric(alpha) - 1

R/trees.R

@@ -13,6 +13,7 @@
 #' not to be a tree.
 #'
 #' @param graph An igraph graph object
+#' @inheritParams rlang::args_dots_empty
 #' @param mode Whether to consider edge directions in a directed graph.
 #'   \sQuote{all} ignores edge directions; \sQuote{out} requires edges to be
 #'   oriented outwards from the root, \sQuote{in} requires edges to be oriented
@@ -42,9 +43,32 @@
 #' @export
 is_tree <- function(
   graph,
+  ...,
   mode = c("out", "in", "all", "total"),
   details = FALSE
 ) {
+  # BEGIN GENERATED ARG_HANDLE: is_tree, do not edit, see tools/generate-migrations.R
+  if (...length() > 0L) {
+    .arg_handle <- migrate_recover_args(
+      list(...),
+      current = list(mode = mode, details = details),
+      recover_new = c("mode", "details"),
+      recover_old = c("mode", "details"),
+      match_names = c("mode", "details"),
+      match_to = c("mode", "details"),
+      defaults = list(mode = c("out", "in", "all", "total"), details = FALSE),
+      head_args = c("graph"),
+      fn_name = "is_tree"
+    )
+    list2env(.arg_handle$values, environment())
+    lifecycle::deprecate_soft(
+      "3.0.0",
+      what = I(.arg_handle$what),
+      details = .arg_handle$details
+    )
+  }
+  # END GENERATED ARG_HANDLE
+
   out <- is_tree_impl(
     graph = graph,
     mode = mode,
@@ -72,6 +96,7 @@ is_tree <- function(
 #' to be a forest.
 #'
 #' @param graph An igraph graph object
+#' @inheritParams rlang::args_dots_empty
 #' @param mode Whether to consider edge directions in a directed graph.
 #'   \sQuote{all} ignores edge directions; \sQuote{out} requires edges to be
 #'   oriented outwards from the root, \sQuote{in} requires edges to be oriented
@@ -101,9 +126,32 @@ is_tree <- function(
 #' @export
 is_forest <- function(
   graph,
+  ...,
   mode = c("out", "in", "all", "total"),
   details = FALSE
 ) {
+  # BEGIN GENERATED ARG_HANDLE: is_forest, do not edit, see tools/generate-migrations.R
+  if (...length() > 0L) {
+    .arg_handle <- migrate_recover_args(
+      list(...),
+      current = list(mode = mode, details = details),
+      recover_new = c("mode", "details"),
+      recover_old = c("mode", "details"),
+      match_names = c("mode", "details"),
+      match_to = c("mode", "details"),
+      defaults = list(mode = c("out", "in", "all", "total"), details = FALSE),
+      head_args = c("graph"),
+      fn_name = "is_forest"
+    )
+    list2env(.arg_handle$values, environment())
+    lifecycle::deprecate_soft(
+      "3.0.0",
+      what = I(.arg_handle$what),
+      details = .arg_handle$details
+    )
+  }
+  # END GENERATED ARG_HANDLE
+
   is_forest_impl(
     graph = graph,
     mode = mode,
@@ -148,6 +196,7 @@ to_prufer <- function(graph) {
 #'
 #' @param graph The input graph to sample from. Edge directions are ignored if
 #'   the graph is directed.
+#' @inheritParams rlang::args_dots_empty
 #' @param vid When the graph is disconnected, this argument specifies how to
 #'   handle the situation. When the argument is zero (the default), the sampling
 #'   will be performed component-wise, and the result will be a spanning forest.
@@ -167,7 +216,33 @@ to_prufer <- function(graph) {
 #'
 #' @family trees
 #' @export
-sample_spanning_tree <- function(graph, vid = 0) {
+sample_spanning_tree <- function(
+  graph,
+  ...,
+  vid = 0
+) {
+  # BEGIN GENERATED ARG_HANDLE: sample_spanning_tree, do not edit, see tools/generate-migrations.R
+  if (...length() > 0L) {
+    .arg_handle <- migrate_recover_args(
+      list(...),
+      current = list(vid = vid),
+      recover_new = c("vid"),
+      recover_old = c("vid"),
+      match_names = c("vid"),
+      match_to = c("vid"),
+      defaults = list(vid = 0),
+      head_args = c("graph"),
+      fn_name = "sample_spanning_tree"
+    )
+    list2env(.arg_handle$values, environment())
+    lifecycle::deprecate_soft(
+      "3.0.0",
+      what = I(.arg_handle$what),
+      details = .arg_handle$details
+    )
+  }
+  # END GENERATED ARG_HANDLE
+
   random_spanning_tree_impl(
     graph = graph,
     vid = vid

tests/testthat/test-decomposition.R

@@ -40,3 +40,41 @@ test_that("is_chordal works", {
   ic2 <- is_chordal(g2, fillin = TRUE)
   expect_equal(ic2, list(chordal = TRUE, fillin = numeric(), newgraph = NULL))
 })
+
+# ---- ellipsis migration: argument coverage ----------------------------
+
+# The first example graph from the Tarjan-Yannakakis paper, as in the test above.
+tarjan_yannakakis_graph <- function() {
+  graph_from_literal(
+    A - B:C:I, B - A:C:D, C - A:B:E:H, D - B:E:F,
+    E - C:D:F:H, F - D:E:G, G - F:H, H - C:E:G:I,
+    I - A:H
+  )
+}
+
+test_that("is_chordal accepts alpha, alpham1, fillin, and newgraph by name", {
+  g <- tarjan_yannakakis_graph()
+  mc <- max_cardinality(g)
+
+  ic <- is_chordal(
+    g,
+    alpha = mc$alpha,
+    alpham1 = mc$alpham1,
+    fillin = TRUE,
+    newgraph = TRUE
+  )
+  expect_false(ic$chordal)
+  expect_equal(unique(sort(ic$fillin)), c(1, 2, 5, 6, 7, 8))
+  # The triangulated graph adds exactly the fill-in edges and is chordal.
+  expect_vcount(ic$newgraph, vcount(g))
+  expect_ecount(ic$newgraph, ecount(g) + length(ic$fillin) / 2)
+  expect_true(is_chordal(ic$newgraph)$chordal)
+})
+
+test_that("is_chordal recovers legacy positional arguments", {
+  g <- tarjan_yannakakis_graph()
+  mc <- max_cardinality(g)
+  lifecycle::expect_deprecated(res <- is_chordal(g, mc$alpha))
+  expect_identical(res, is_chordal(g, alpha = mc$alpha))
+  expect_false(res$chordal)
+})

tests/testthat/test-trees.R

@@ -227,3 +227,47 @@ test_that("subgraph.edges deprecation", {
   edges <- sample_spanning_tree(g)
   expect_snapshot(subgraph.edges(g, edges, delete.vertices = FALSE))
 })
+
+# ---- ellipsis migration: argument coverage ----------------------------
+
+test_that("is_tree recovers legacy positional arguments", {
+  g <- make_tree(7, 2, mode = "in")
+  # The default mode "out" rejects an in-tree, so the recovered value is visible.
+  expect_false(is_tree(g))
+  lifecycle::expect_deprecated(res <- is_tree(g, "in"))
+  expect_identical(res, is_tree(g, mode = "in"))
+  expect_true(res)
+})
+
+test_that("is_forest returns component roots for the requested mode", {
+  g <- make_tree(3, mode = "in") + make_tree(5, 3, mode = "in")
+
+  res <- is_forest(g, mode = "in", details = TRUE)
+  expect_named(res, c("res", "roots"))
+  expect_true(res$res)
+  expect_equal(ignore_attr = TRUE, res$roots, V(g)[c(1, 4)])
+
+  # The same forest fails the check against the opposite orientation.
+  res_out <- is_forest(g, mode = "out", details = TRUE)
+  expect_named(res_out, c("res", "roots"))
+  expect_false(res_out$res)
+})
+
+test_that("is_forest recovers legacy positional arguments", {
+  g <- make_graph(c(1, 2, 2, 3, 2, 4, 5, 4), n = 6, directed = TRUE)
+  # The default mode "out" rejects this graph, so the recovered value is visible.
+  expect_false(is_forest(g))
+  lifecycle::expect_deprecated(res <- is_forest(g, "all"))
+  expect_identical(res, is_forest(g, mode = "all"))
+  expect_true(res)
+})
+
+test_that("sample_spanning_tree recovers legacy positional arguments", {
+  g <- make_full_graph(8) %du% make_full_graph(5)
+  lifecycle::expect_deprecated(
+    res <- igraph_with_seed(42, sample_spanning_tree(g, 9))
+  )
+  # vid = 9 selects the 5-vertex component, so its spanning tree has 4 edges.
+  expect_length(res, 4)
+  expect_equal(res, igraph_with_seed(42, sample_spanning_tree(g, vid = 9)))
+})

tools/migrations/trees.R

@@ -0,0 +1,50 @@
+# Argument-signature migrations: trees
+# Schema: see tools/migrations.R. Regenerate with:
+#   Rscript tools/generate-migrations.R
+
+migrations <- list(
+  is_chordal = list(
+    old = function(graph, alpha, alpham1, fillin, newgraph) {},
+    new = function(
+      graph,
+      ...,
+      alpha = NULL,
+      alpham1 = NULL,
+      fillin = FALSE,
+      newgraph = FALSE
+    ) {},
+    when = "3.0.0"
+  ),
+
+  is_forest = list(
+    old = function(graph, mode, details) {},
+    new = function(
+      graph,
+      ...,
+      mode = c("out", "in", "all", "total"),
+      details = FALSE
+    ) {},
+    when = "3.0.0"
+  ),
+
+  is_tree = list(
+    old = function(graph, mode, details) {},
+    new = function(
+      graph,
+      ...,
+      mode = c("out", "in", "all", "total"),
+      details = FALSE
+    ) {},
+    when = "3.0.0"
+  ),
+
+  sample_spanning_tree = list(
+    old = function(graph, vid) {},
+    new = function(
+      graph,
+      ...,
+      vid = 0
+    ) {},
+    when = "3.0.0"
+  )
+)