feat: retire `attr` in favor of `weights` for adjacency matrix functions by schochastics · Pull Request #2677 · igraph/rigraph

schochastics · 2026-05-28T17:34:36Z

Summary

Resolves milestone #30 — retire attr for weight in one PR. Bundles the API change with the dependent centrality function migrations so no temporary translation shims are needed.

Closes: #906, #903, #910 — partially closes #1137 (the sparse path already worked; this PR audits and adds parity tests).

Changes

1. `as_adjacency_matrix()` / `as_adj()` (#906)

New weights argument follows the standard rigraph weight convention:

`weights =`	Behavior
`NULL` (default)	Use the `weight` edge attribute if present, else unweighted
`NA`	Explicitly unweighted (ignores any `weight` attribute)
`<numeric vector>`	Use directly as edge weights (length must equal `ecount()`)
`<character scalar>`	Edge attribute name (must be numeric/logical)

The legacy attr argument is now lifecycle::deprecate_warn()-ed against igraph 3.0.0. When attr is supplied it is forwarded into weights, so all existing user code keeps working with a soft warning.

2. `as_biadjacency_matrix()`

Same migration mirrored for the bipartite sibling. The C binding R_igraph_get_biadjacency doesn't have a weights slot, so the four-mode resolution happens entirely in R via the helper functions.

3. `alpha_centrality()` (#910)

The previous implementation laundered a numeric weights vector through a set_edge_attr(graph, "weight", value = ...) mutation just to use the old attr = API. That workaround is gone — both the dense and sparse helpers now pass weights straight to as_adjacency_matrix(). This also structurally fixes the bug class behind #915 (custom weight attribute names other than "weight").

4. `power_centrality()` (#903)

Gained a weights argument. Previously it silently dropped edge weights, returning identical results for a weighted graph and its unweighted version. Weights now thread through both bonpow.dense() and bonpow.sparse(). Drive-by: the sparse path used degree(graph, mode = "out") where the dense path used rowSums(d); these agree only for unweighted graphs. The sparse path now uses Matrix::rowSums(d) for consistency.

5. Internal call site

R/indexing.R:269 (the [ operator on graphs) now passes weights = if (is.null(attr)) NA else attr instead of attr = attr — preserving its existing "unweighted by default" semantics rather than tripping the new auto-pickup.

6. Tests

Migrated existing legacy attr = test calls to weights = (test-conversion.R, test-incidence.R, test-structural-properties.R, plus the matching snapshot).
Added focused coverage for all four weights modes, length/type errors, dense/sparse parity for arbitrary numeric vectors, and the new deprecation warnings (with snapshots).
Added power_centrality() reprex from power_centrality() should support edge weights #903 (weighted graph equivalent to unweighted graph with parallel edges).
Added regression test for alpha_centrality() crashes when weights argument is a string other than "weight" #915 (custom weight attribute name).

Soft-breaking change

as_adjacency_matrix(g) on a graph that already has a weight edge attribute now returns a weighted matrix by default. Old behavior: pass weights = NA. This brings the function into line with the rest of rigraph (shortest_paths(), cluster_leiden(), etc.).

Out of scope (followups)

Deprecated wrappers get.adjacency(), get.incidence(), as_incidence_matrix() are intentionally not touched. They forward attr = attr to the migrated functions, so calling them now emits the outer "function is deprecated" warning plus the new attr = warning. That doubling is acceptable; users are supposed to be migrating off these anyway.
A future PR will flip lifecycle::deprecate_warn() → lifecycle::deprecate_stop() for attr = after one release cycle on CRAN.

Test plan

devtools::test() — 0 failures across the full suite (NOT_CRAN=true).

Targeted REPL spot-check:

g <- make_ring(5); E(g)$weight <- 1:5
as_adjacency_matrix(g)                    # weighted (new default)
as_adjacency_matrix(g, weights = NA)      # unweighted
as_adjacency_matrix(g, weights = 10:14)   # explicit vector
as_adjacency_matrix(g, weights = "weight")# attribute name
as_adjacency_matrix(g, attr = "weight")   # deprecated; warns; weighted

🤖 Generated with Claude Code

Introduces a `weights` argument on `as_adjacency_matrix()` / `as_adj()` and `as_biadjacency_matrix()` following the standard rigraph weight convention (`NULL` auto-picks `weight`, `NA` forces unweighted, numeric vector is used directly, character scalar names an attribute). The legacy `attr` argument is now soft-deprecated via `lifecycle::deprecate_warn()`. Centrality functions that previously routed weights through a temporary `weight` edge attribute have been simplified to use the new vector path directly: `alpha_centrality()` drops the graph-mutation workaround (#910), and `power_centrality()` gains a `weights` argument (#903). This also structurally fixes the bug class behind #915. Closes: #906, #903, #910, partially #1137

…weights

Run: https://github.com/igraph/rigraph/actions/runs/26591333825

schochastics · 2026-05-28T18:04:03Z

To me this looks good. @krlmlr do we run revdeps on each potentially breaking PR? If so, this definitely needs it 😬

krlmlr

I'm not sure I like it. Could we keep two arguments, attr and weight, and resolve them in unison?

The previous commit hand-edited four Rd files for the functions whose signatures changed. The package's CI regenerates `man/*.Rd` from the roxygen comments in `R/`, so committing those edits adds nothing — the source-of-truth roxygen blocks are already updated in the same commit. Removing them keeps the diff minimal and avoids divergence from the canonical generator. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

schochastics · 2026-05-28T18:25:02Z

I'm not sure I like it. Could we keep two arguments, attr and weight, and resolve them in unison?

You mean no deprecation warning for attr and silently forward attr to weight?

krlmlr

Took another look. Let's discuss.

krlmlr · 2026-05-28T18:39:27Z

+  weights = NULL,
+  attr = deprecated(),
  names = TRUE,
  sparse = FALSE


How about adding an ellipsis after types, while we're at it? Adding arguments mid-signature calls for trouble.

I add the ellipses but I think this can also lead some (soft) breaking changes

…nd extract edge_attr_as_weights helper; add `...` for future extensions to as_adjacency_matrix and as_biadjacency_matrix

Run: https://github.com/igraph/rigraph/actions/runs/26598981879

krlmlr

Thanks, great! I skimmed the patch, let's accumulate breaking PRs (by merging into main) and run revdepchecks in bulk.

krlmlr · 2026-05-29T03:44:03Z

  sparse = igraph_opt("sparsematrices")
 ) {
  ensure_igraph(graph)
+  rlang::check_dots_empty()


It is possible to implement this in a non-breaking way. I did this for dm 1.0.0, the idea is to detect non-empty dots and create a function that matches the args according to the old signature. We can do this depending on revdepcheck results (maybe faster now), or adopt this practice as a rule (allows us to easily add dots without breaking downstream other than soft-deprecation warnings).

I like your suggestion because I had some headaches over this. Let me try and come up with something

hmm @krlmlr Claude tells me that what we are doing here is what dm 1.0.0 does

We had a substantial interface change for dm_filter() , I think this is the most relevant one. We don't have to do it exactly like with that function, but a design along these lines might be useful.

I used dm_filter as inspiration. There is now a function that handles this similarly here

@schochastics @krlmlr side-note, this would be a very nice short blog post. Adding the empty dot checks in a non breaking way.

…d as_biadjacency_matrix() via `...` with soft deprecation

krlmlr

Thanks. I had hoped we could recover other unnamed arguments as well. I wonder if we can create a very generic helper function that accepts the old signature and returns a list of values from a call with the new signature that we can then deconstruct again. That function would also issue lifecycle warnings. With such a function, we could add dots everywhere in one swoop: once the helper exists, Claude can embed it everywhere.

as_biadjacency_matrix <- function(graph, types, ..., weights, attrs, names, sparse) {
  args <- handle_args(
    graph, types, ..., weights, attrs, names, sparse,
    .old_signature = list(graph, types, attrs, names, sparse)
  )
  graph <- args$graph
  types <- args$types
  ...
}

We could also create a dedicated helper for each such function (deterministic code generation) to simplify debugging and enable custom changes in the helper code.

as_biadjacency_matrix <- function(graph, types, ..., weights, attrs, names, sparse) {
  args <- handle_args_as_biadjacency_matrix(
    graph, types, ..., weights, attrs, names, sparse
  )
  graph <- args$graph
  types <- args$types
  ...
}

If we do this, we can do a separate PR before this. Well in scope for 3.0.0 if we agree on that; an alternative is a hard break and dealing with the revdepchecks.

krlmlr · 2026-05-31T07:04:10Z

  ensure_igraph(graph)

+  user_env <- rlang::caller_env()
+  recovered_attr <- recover_positional_attr(


How does this code handle as_adjacency_matrix(graph, type, attr, names = letters) ?

maelle

a few (most off PR topic?) comments, thank you!!

maelle · 2026-06-02T08:27:17Z

  nodes <- as_igraph_vs(graph, nodes)
  if (sparse) {
-    res <- bonpow.sparse(graph, nodes, loops, exponent, rescale, tol)
+    res <- bonpow.sparse(


since bonpow.sparse() is not exported, could we give it a better name? Or do we want to keep dot names for unexported functions?

maelle · 2026-06-02T08:28:43Z

@@ -1966,25 +1986,7 @@ alpha.centrality.dense <- function(
  exo <- rep(exo, length.out = vcount(graph))


What does exo mean? In my French-speaking head it's an abbreviation of exercise so I'm confused.

maelle · 2026-06-02T08:29:01Z

-  }
-
-  d <- t(as_adjacency_matrix(graph, attr = attr, sparse = FALSE))
+  d <- t(as_adjacency_matrix(graph, weights = weights, sparse = FALSE))


better name than d?

maelle · 2026-06-02T08:29:24Z

-  }
-
-  M <- Matrix::t(as_adjacency_matrix(graph, attr = attr, sparse = TRUE))
+  M <- Matrix::t(as_adjacency_matrix(graph, weights = weights, sparse = TRUE))


or are these single letters sort of convention?

maelle · 2026-06-02T08:30:10Z

 #
 ###################################################################

+# Resolve the user-facing `weights` argument into a numeric vector


is this now valid in all of igraph?

maelle · 2026-06-02T08:38:29Z

+        call = call
+      )
+    }
+    if (!weights %in% edge_attr_names(graph)) {


I can't wait for %notin% to be old enough for us to use it.

maelle · 2026-06-02T08:38:47Z

+    return(edge_attr_as_weights(graph, weights, call))
+  }
+
+  if (!is.numeric(weights) && !is.logical(weights)) {


couldn't we check the types earlier?

maelle · 2026-06-02T08:39:32Z

+# would otherwise hard-fail via rlang::check_dots_empty().
+#
+# Dispatcher is dm-style: discriminate on whether dots are named or not
+# (cynkra/dm R/filter-dm.R, dm_filter_api1()). Named extras stay an error


better to use a GitHub permalink?

maelle · 2026-06-02T08:40:40Z

-#'   also allowed. The reason for the difference is that the `Matrix`
-#'   package does not support character sparse matrices yet.
+#' @param ... These dots are for future extensions and must be empty.
+#' @param weights One of the following:


so this will be inherited all over the place?

maelle · 2026-06-02T08:41:14Z

+#'   is included in the matrix.
+#' @param attr `r lifecycle::badge("deprecated")` Use `weights` instead. If
+#'   supplied, the value is forwarded to `weights` as a character edge
+#'   attribute name.


but this will be removed in future versions

schochastics and others added 3 commits May 28, 2026 19:33

Merge remote-tracking branch 'origin/main' into feat/retire-attr-for-…

8792b50

…weights

chore: Auto-update from GitHub Actions

c46a556

Run: https://github.com/igraph/rigraph/actions/runs/26591333825

schochastics added the hackathon-topic 📆 label May 28, 2026

schochastics requested review from krlmlr and maelle May 28, 2026 17:59

krlmlr reviewed May 28, 2026

View reviewed changes

Comment thread R/conversion.R Outdated

Comment thread R/conversion.R Outdated

krlmlr reviewed May 28, 2026

View reviewed changes

schochastics and others added 4 commits May 28, 2026 21:13

document

f1b705d

Refactor resolve_edge_weights to handle deprecated attr parameter a…

97bbcd6

…nd extract edge_attr_as_weights helper; add `...` for future extensions to as_adjacency_matrix and as_biadjacency_matrix

doc

47ef17f

chore: Auto-update from GitHub Actions

0df5d1f

Run: https://github.com/igraph/rigraph/actions/runs/26598981879

krlmlr reviewed May 29, 2026

View reviewed changes

krlmlr approved these changes May 29, 2026

View reviewed changes

schochastics added 2 commits May 29, 2026 09:13

Unify weights parameter docs via @inheritParams as_adjacency_matrix

9d6f0d3

Recover legacy positional attr argument in as_adjacency_matrix() an…

d783eec

…d as_biadjacency_matrix() via `...` with soft deprecation

krlmlr reviewed May 31, 2026

View reviewed changes

schochastics mentioned this pull request Jun 1, 2026

feat: add tooling for parameter migration #2684

Merged

maelle reviewed Jun 2, 2026

View reviewed changes

Merge branch 'main' into feat/retire-attr-for-weights

c662399

		@@ -1966,25 +1986,7 @@ alpha.centrality.dense <- function(
		exo <- rep(exo, length.out = vcount(graph))

Uh oh!

Conversation

schochastics commented May 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

1. as_adjacency_matrix() / as_adj() (#906)

2. as_biadjacency_matrix()

3. alpha_centrality() (#910)

4. power_centrality() (#903)

5. Internal call site

6. Tests

Soft-breaking change

Out of scope (followups)

Test plan

Uh oh!

schochastics commented May 28, 2026

Uh oh!

krlmlr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

schochastics commented May 28, 2026

Uh oh!

krlmlr left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

krlmlr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

schochastics May 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

krlmlr left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

maelle left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

schochastics commented May 28, 2026 •

edited

Loading

1. `as_adjacency_matrix()` / `as_adj()` (#906)

2. `as_biadjacency_matrix()`

3. `alpha_centrality()` (#910)

4. `power_centrality()` (#903)

schochastics May 29, 2026 •

edited

Loading