Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 9 additions & 7 deletions R/centrality.R
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ subgraph.centrality <- function(graph, diag = FALSE) {
#' `page.rank()` was renamed to [page_rank()] to create a more
#' consistent API.
#' @inheritParams page_rank
#' @param algo `r lifecycle::badge("deprecated")` Use `algorithm` in
#' [page_rank()] instead.
#' @keywords internal
#' @export
page.rank <- function(
Expand All @@ -42,7 +44,7 @@ page.rank <- function(
lifecycle::deprecate_warn("2.0.0", "page.rank()", "page_rank()")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When will we get rid of those functions? Could we try deprecate_stop() in a PR? They're dead weight we're carrying around.

page_rank(
graph = graph,
algo = algo,
algorithm = algo,
vids = vids,
directed = directed,
damping = damping,
Expand Down Expand Up @@ -1884,7 +1886,7 @@ hub_score <- function(
#'
#' @param graph The graph object.
#' @inheritParams rlang::args_dots_empty
#' @param algo Character scalar, which implementation to use to carry out the
#' @param algorithm Character scalar, which implementation to use to carry out the
#' calculation. The default is `"prpack"`, which uses the PRPACK library
#' (<https://github.com/dgleich/prpack>) to calculate PageRank scores
#' by solving a set of linear equations. This is a new implementation in igraph
Expand Down Expand Up @@ -1953,7 +1955,7 @@ hub_score <- function(
page_rank <- function(
graph,
...,
algo = c("prpack", "arpack"),
algorithm = c("prpack", "arpack"),
vids = NULL,
directed = TRUE,
damping = 0.85,
Expand All @@ -1975,7 +1977,7 @@ page_rank <- function(
cli::cli_abort(base::c("Unexpected argument passed to {.fn page_rank}: {.arg {(.arg_extra)}}.", i = "Arguments after {.arg ...} must be spelled out in full."), call = base::parent.frame())
}
base::c(
if (!base::missing(algo)) base::list(algo = algo),
if (!base::missing(algo)) base::list(algorithm = algo),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we namespace base functions?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (!base::missing(vids)) base::list(vids = vids),
if (!base::missing(directed)) base::list(directed = directed),
if (!base::missing(damping)) base::list(damping = damping),
Expand All @@ -1988,7 +1990,7 @@ page_rank <- function(
if (base::length(.arg_handle) > 0L) {
.arg_names <- base::names(.arg_handle)
.arg_conflict <- base::intersect(.arg_names, base::c(
if (!base::missing(algo)) "algo",
if (!base::missing(algorithm)) "algorithm",
if (!base::missing(vids)) "vids",
if (!base::missing(directed)) "directed",
if (!base::missing(damping)) "damping",
Expand All @@ -2002,7 +2004,7 @@ page_rank <- function(
"3.0.0",
what = base::I("Calling `page_rank()` with positional or abbreviated arguments"),
details = base::c(
i = base::paste0("Detected call: page_rank(", base::paste(base::c("graph", .arg_names), collapse = ", "), ")"),
i = base::paste0("Detected call: page_rank(", base::paste(base::c("graph", base::c(algorithm = "algo", vids = "vids", directed = "directed", damping = "damping", personalized = "personalized", weights = "weights", options = "options")[.arg_names]), collapse = ", "), ")"),
i = base::paste0("Use instead: page_rank(", base::paste(base::c("graph", base::paste0(.arg_names, " = ")), collapse = ", "), ")")
)
)
Expand All @@ -2016,7 +2018,7 @@ page_rank <- function(

personalized_pagerank_impl(
graph = graph,
algo = algo,
algo = algorithm,
vids = vids,
directed = directed,
damping = damping,
Expand Down
2 changes: 0 additions & 2 deletions R/migration-fixture.R
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@ migration_fixture <- function(
# BEGIN GENERATED ARG_HANDLE: migration_fixture, do not edit, see tools/generate-migrations.R
# fmt: skip
if (...length() > 0L) {
.arg_ambiguous <- base::intersect(base::names(base::substitute(...())), base::c("w", "we", "wei", "weig", "weigh"))
if (base::length(.arg_ambiguous) > 0L) cli::cli_abort("Argument {.arg {(.arg_ambiguous[[1L]])}} matches multiple arguments of {.fn migration_fixture}.")
# Pre-3.0.0 signature: migration_fixture(graph, n, weight, kind, directed)
.old_signature <- function(weight, kind, directed, ...) {
if (...length() > 0L) {
Expand Down
26 changes: 13 additions & 13 deletions R/structural-properties.R
Original file line number Diff line number Diff line change
Expand Up @@ -3216,7 +3216,7 @@ topo_sort <- function(
#' `NULL`, then the edge attribute is used automatically. The goal of
#' the feedback arc set problem is to find a feedback arc set with the smallest
#' total weight.
#' @param algo Specifies the algorithm to use. \dQuote{`exact_ip`} solves
#' @param algorithm Specifies the algorithm to use. \dQuote{`exact_ip`} solves
#' the feedback arc set problem with an exact integer programming algorithm that
#' guarantees that the total weight of the removed edges is as small as possible.
#' \dQuote{`approx_eades`} uses a fast (linear-time) approximation
Expand All @@ -3236,12 +3236,12 @@ topo_sort <- function(
#'
#' g <- sample_gnm(20, 40, directed = TRUE)
#' feedback_arc_set(g)
#' feedback_arc_set(g, algo = "approx_eades")
#' feedback_arc_set(g, algorithm = "approx_eades")
feedback_arc_set <- function(
graph,
...,
weights = NULL,
algo = c("approx_eades", "exact_ip")
algorithm = c("approx_eades", "exact_ip")
) {
# BEGIN GENERATED ARG_HANDLE: feedback_arc_set, do not edit, see tools/generate-migrations.R
# fmt: skip
Expand All @@ -3256,23 +3256,23 @@ feedback_arc_set <- function(
}
base::c(
if (!base::missing(weights)) base::list(weights = weights),
if (!base::missing(algo)) base::list(algo = algo)
if (!base::missing(algo)) base::list(algorithm = algo)
)
}
.arg_handle <- .old_signature(...)
if (base::length(.arg_handle) > 0L) {
.arg_names <- base::names(.arg_handle)
.arg_conflict <- base::intersect(.arg_names, base::c(
if (!base::missing(weights)) "weights",
if (!base::missing(algo)) "algo"
if (!base::missing(algorithm)) "algorithm"
))
if (base::length(.arg_conflict) > 0L) cli::cli_abort(base::c("Argument {.arg {(.arg_conflict)}} of {.fn feedback_arc_set} was supplied more than once.", i = "Pass it exactly once, by its new name {.arg {(.arg_conflict)}}."))
base::list2env(.arg_handle, base::environment())
lifecycle::deprecate_soft(
"3.0.0",
what = base::I("Calling `feedback_arc_set()` with positional or abbreviated arguments"),
details = base::c(
i = base::paste0("Detected call: feedback_arc_set(", base::paste(base::c("graph", .arg_names), collapse = ", "), ")"),
i = base::paste0("Detected call: feedback_arc_set(", base::paste(base::c("graph", base::c(weights = "weights", algorithm = "algo")[.arg_names]), collapse = ", "), ")"),
i = base::paste0("Use instead: feedback_arc_set(", base::paste(base::c("graph", base::paste0(.arg_names, " = ")), collapse = ", "), ")")
)
)
Expand All @@ -3283,7 +3283,7 @@ feedback_arc_set <- function(
feedback_arc_set_impl(
graph = graph,
weights = weights,
algo = algo
algo = algorithm
)
}

Expand All @@ -3303,7 +3303,7 @@ feedback_arc_set <- function(
#' `NULL`, then the vertex attribute is used automatically. The goal of
#' the feedback vertex set problem is to find a feedback vertex set with
#' the smallest total weight.
#' @param algo Specifies the algorithm to use. Currently, \dQuote{`exact_ip`},
#' @param algorithm Specifies the algorithm to use. Currently, \dQuote{`exact_ip`},
#' which solves the feedback vertex set problem with an exact integer
#' programming approach, is the only option.
#' @return A vertex sequence (by default, but see the `return.vs.es` option
Expand All @@ -3320,7 +3320,7 @@ feedback_vertex_set <- function(
graph,
...,
weights = NULL,
algo = c("exact_ip")
algorithm = c("exact_ip")
) {
# BEGIN GENERATED ARG_HANDLE: feedback_vertex_set, do not edit, see tools/generate-migrations.R
# fmt: skip
Expand All @@ -3335,23 +3335,23 @@ feedback_vertex_set <- function(
}
base::c(
if (!base::missing(weights)) base::list(weights = weights),
if (!base::missing(algo)) base::list(algo = algo)
if (!base::missing(algo)) base::list(algorithm = algo)
)
}
.arg_handle <- .old_signature(...)
if (base::length(.arg_handle) > 0L) {
.arg_names <- base::names(.arg_handle)
.arg_conflict <- base::intersect(.arg_names, base::c(
if (!base::missing(weights)) "weights",
if (!base::missing(algo)) "algo"
if (!base::missing(algorithm)) "algorithm"
))
if (base::length(.arg_conflict) > 0L) cli::cli_abort(base::c("Argument {.arg {(.arg_conflict)}} of {.fn feedback_vertex_set} was supplied more than once.", i = "Pass it exactly once, by its new name {.arg {(.arg_conflict)}}."))
base::list2env(.arg_handle, base::environment())
lifecycle::deprecate_soft(
"3.0.0",
what = base::I("Calling `feedback_vertex_set()` with positional or abbreviated arguments"),
details = base::c(
i = base::paste0("Detected call: feedback_vertex_set(", base::paste(base::c("graph", .arg_names), collapse = ", "), ")"),
i = base::paste0("Detected call: feedback_vertex_set(", base::paste(base::c("graph", base::c(weights = "weights", algorithm = "algo")[.arg_names]), collapse = ", "), ")"),
i = base::paste0("Use instead: feedback_vertex_set(", base::paste(base::c("graph", base::paste0(.arg_names, " = ")), collapse = ", "), ")")
)
)
Expand All @@ -3362,7 +3362,7 @@ feedback_vertex_set <- function(
feedback_vertex_set_impl(
graph = graph,
weights = weights,
algo = algo
algo = algorithm
)
}

Expand Down
6 changes: 3 additions & 3 deletions man/feedback_arc_set.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions man/feedback_vertex_set.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 2 additions & 8 deletions man/page.rank.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions man/page_rank.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions tests/testthat/_snaps/centrality.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,3 +59,13 @@
Error in `arpack()`:
! Can't use unkown ARPACK options: unknown_thing1, unknown_thing2

# page_rank(algo = ) is deprecated but still works

Code
res_legacy <- page_rank(star, algo = "prpack")
Condition
Warning:
Calling `page_rank()` with positional or abbreviated arguments was deprecated in igraph 3.0.0.
i Detected call: page_rank(graph, algo)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
i Detected call: page_rank(graph, algo)
i Detected call: page_rank(graph, algo = )

I wish this looked like this so that the two lines might have real parallelism.

i Use instead: page_rank(graph, algorithm = )

34 changes: 26 additions & 8 deletions tests/testthat/_snaps/migration-fixture.md
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,32 @@
[1] TRUE


---

Code
migration_fixture("g", 5, weig = 1)
Condition
Warning:
Calling `migration_fixture()` with positional or abbreviated arguments was deprecated in igraph 3.0.0.
i Detected call: migration_fixture(graph, n, weight)
i Use instead: migration_fixture(graph, n, weights = )
Output
$graph
[1] "g"

$n
[1] 5

$weights
[1] 1

$type
[1] "out"

$directed
[1] FALSE


---

Code
Expand Down Expand Up @@ -350,14 +376,6 @@

# error message snapshots

Code
migration_fixture("g", 5, weig = 1)
Condition
Error in `migration_fixture()`:
! Argument `weig` matches multiple arguments of `migration_fixture()`.

---

Code
migration_fixture("g", 5, foo = 1)
Condition
Expand Down
24 changes: 21 additions & 3 deletions tests/testthat/test-centrality.R
Original file line number Diff line number Diff line change
Expand Up @@ -1074,12 +1074,12 @@ test_that("page_rank() covers migrated tail args and positional recovery", {
# The weight attribute is a decoy that the explicit `weights` must override.
E(star)$weight <- c(10, rep(1, 8))

# `algo` keeps its default value:
# `algorithm` keeps its default value:
# non-default values select the legacy ARPACK implementation,
# and `options` is only consumed by that implementation.
res <- page_rank(
star,
algo = "prpack",
algorithm = "prpack",
vids = V(star)[1:5],
directed = FALSE,
damping = 0.9,
Expand All @@ -1106,7 +1106,25 @@ test_that("page_rank() covers migrated tail args and positional recovery", {
lifecycle::expect_deprecated(
res_legacy <- page_rank(star, "prpack")
)
expect_identical(res_legacy, page_rank(star, algo = "prpack"))
expect_identical(res_legacy, page_rank(star, algorithm = "prpack"))
})

test_that("page_rank(algo = ) is deprecated but still works", {
rlang::local_options(lifecycle_verbosity = "warning")
star <- make_star(10, mode = "undirected")
expect_snapshot(
res_legacy <- page_rank(star, algo = "prpack")
)
expect_identical(res_legacy, page_rank(star, algorithm = "prpack"))
# `alg` prefixes both `algo` and `algorithm`, but they are the same argument,
# so it is recovered rather than rejected as ambiguous.
lifecycle::expect_deprecated(res_abbrev <- page_rank(star, alg = "prpack"))
expect_identical(res_abbrev, page_rank(star, algorithm = "prpack"))
# `d` really is ambiguous (`damping` vs `directed`) and stays an error.
expect_error(
page_rank(star, d = 0.5),
"matches multiple arguments"
)
})

test_that("strength() covers migrated tail args and positional recovery", {
Expand Down
Loading
Loading