feat: Add attribute combination support to graph operators by schochastics · Pull Request #2676 · igraph/rigraph

schochastics · 2026-05-28T11:37:31Z

…tion, compose, disjoint_union)

schochastics · 2026-05-28T11:59:26Z

devtools::document() doesnt work locally for me

krlmlr · 2026-06-04T08:49:27Z

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

-igraph.i.attribute.combination <- function(comb) {
+igraph.i.attribute.combination <- function(comb, allow_rename = FALSE) {


Split into two functions, one for a scalar, and the list function as a simple wrapper?

krlmlr · 2026-06-04T08:52:36Z

+  default_comb <- if (length(default_idx) > 0) {
+    comb[[default_idx[1]]]
+  } else {
+    "rename"


Suggested change

"rename"

# Backward-compatible standard

"rename"

krlmlr · 2026-06-04T09:12:07Z

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

-rename.attr.if.needed <- function(
+combine.attrs <- function(


Keep rename_attr_if_needed() and use it only for those attributes where the action is "rename"?

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

github-actions · 2026-06-10T07:05:06Z

This is how benchmark results would change (along with a 95% confidence interval in relative change) if c26ab02 is merged into main:

✔️as_adjacency_matrix: 777ms -> 779ms [-0.27%, +0.78%]
✔️as_biadjacency_matrix: 777ms -> 779ms [-0.47%, +1.17%]
✔️as_data_frame_both: 1.61ms -> 1.59ms [-2.88%, +1.5%]
✔️as_long_data_frame: 4ms -> 4ms [-1.29%, +1.34%]
✔️es_attr_filter: 2.8ms -> 2.82ms [-1.01%, +2.48%]
✔️graph_from_adjacency_matrix: 147ms -> 148ms [-0.73%, +1.43%]
✔️graph_from_data_frame: 3.56ms -> 3.56ms [-1.83%, +1.32%]
✔️vs_attr_filter: 1.65ms -> 1.63ms [-3.3%, +0.25%]
✔️vs_by_name: 1.06ms -> 1.06ms [-2.31%, +2.11%]
Further explanation regarding interpretation and methodology can be found in the documentation.

…lper Address review feedback on #2676: - Rename the new graph.attr.comb/vertex.attr.comb/edge.attr.comb arguments of union()/intersection()/compose()/disjoint_union() to snake_case (these args are new in this PR, so no deprecation is needed). - Extract rename_attr_if_needed() back out of combine.attrs() for the "rename" strategy, preserving the historical overwrite-on-clash semantics under chains of %du%. - Add a clarifying comment on the backward-compatible "rename" default. - Move the make_named_pair() test helper into helper.R. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…tted names Follow-up to review feedback on #2676 ("clean up edge.attr.comb"): - simplify(), as_undirected() and contract() gain snake_case edge_attr_comb / vertex_attr_comb arguments via the argument-migration infrastructure (tools/migrations.R). The old dotted names still work and emit a single lifecycle::deprecate_soft() warning. - Rename the matching igraph option keys edge.attr.comb -> edge_attr_comb and vertex.attr.comb -> vertex_attr_comb, with back-compatible aliasing in igraph_opt()/igraph_options(); the dotted keys still read and set, soft- deprecated. Update the stimulus codegen default (types-RR.yaml) and the regenerated *_impl defaults to match. Deprecated wrappers (as.undirected(), contract.vertices()) are left frozen; their snapshot now records the resulting layered deprecation warnings. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

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

github-actions · 2026-06-10T10:43:04Z

This is how benchmark results would change (along with a 95% confidence interval in relative change) if b58da44 is merged into main:

✔️as_adjacency_matrix: 731ms -> 728ms [-1.46%, +0.89%]
✔️as_biadjacency_matrix: 735ms -> 740ms [-0.52%, +1.76%]
✔️as_data_frame_both: 1.51ms -> 1.5ms [-2.15%, +1.45%]
✔️as_long_data_frame: 4.06ms -> 4.05ms [-3.39%, +2.93%]
✔️es_attr_filter: 2.9ms -> 2.93ms [-3.59%, +5.69%]
✔️graph_from_adjacency_matrix: 118ms -> 119ms [-0.35%, +2.21%]
✔️graph_from_data_frame: 3.54ms -> 3.58ms [-1.51%, +4.05%]
✔️vs_attr_filter: 1.66ms -> 1.66ms [-2.81%, +3.13%]
✔️vs_by_name: 1.05ms -> 1.09ms [-4.13%, +11.57%]
Further explanation regarding interpretation and methodology can be found in the documentation.

krlmlr · 2026-06-11T09:04:18Z

+  "vertex_attr_comb" = list(name = "concat", "ignore"),
+  "edge_attr_comb" = list(weight = "sum", name = "concat", "ignore"),


"graph_attr_comb" missing?

…e combination in operators The `graph_attr_comb` parameter in `union()`, `intersection()`, `disjoint_union()`, and `compose()` now defaults to `igraph_opt("graph_attr_comb")` (which is `"rename"` by default) instead of a hard-coded `"rename"`. This allows users to globally configure graph attribute combination behavior via `igraph_options()`.

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

github-actions · 2026-06-16T19:16:16Z

This is how benchmark results would change (along with a 95% confidence interval in relative change) if 6d94668 is merged into main:

🚀as_adjacency_matrix: 753ms -> 714ms [-5.86%, -4.62%]
🚀as_biadjacency_matrix: 760ms -> 719ms [-6.08%, -4.74%]
✔️as_data_frame_both: 1.53ms -> 1.55ms [-1.38%, +3.28%]
✔️as_long_data_frame: 3.91ms -> 3.9ms [-1.4%, +1.38%]
✔️es_attr_filter: 2.72ms -> 2.76ms [-1.17%, +4.01%]
🚀graph_from_adjacency_matrix: 119ms -> 112ms [-6.8%, -4.45%]
❗🐌graph_from_data_frame: 3.41ms -> 3.46ms [+0.56%, +2.73%]
✔️vs_attr_filter: 1.59ms -> 1.61ms [-4.11%, +6.69%]
✔️vs_by_name: 1.02ms -> 1.02ms [-2.37%, +1.64%]
Further explanation regarding interpretation and methodology can be found in the documentation.

schochastics · 2026-06-16T19:30:44Z

@maelle it is now *_combine ;-)

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

github-actions · 2026-06-16T19:44:03Z

This is how benchmark results would change (along with a 95% confidence interval in relative change) if a85a78c is merged into main:

✔️as_adjacency_matrix: 680ms -> 679ms [-0.8%, +0.74%]
❗🐌as_biadjacency_matrix: 678ms -> 683ms [+0.03%, +1.19%]
✔️as_data_frame_both: 1.49ms -> 1.48ms [-2.85%, +1.13%]
✔️as_long_data_frame: 3.94ms -> 3.92ms [-2.14%, +1.01%]
✔️es_attr_filter: 2.7ms -> 2.72ms [-1.14%, +2.69%]
✔️graph_from_adjacency_matrix: 110ms -> 110ms [-1.23%, +1.6%]
✔️graph_from_data_frame: 3.32ms -> 3.4ms [-1.65%, +6.38%]
✔️vs_attr_filter: 1.5ms -> 1.5ms [-2.3%, +2.33%]
✔️vs_by_name: 942µs -> 946µs [-1.47%, +2.21%]
Further explanation regarding interpretation and methodology can be found in the documentation.

maelle · 2026-06-23T07:54:41Z

Yay 🎉

maelle

Thank you!!

maelle · 2026-06-23T07:55:17Z

    cli::cli_warn("Some attributes are duplicated")
  }
+  known_names <- c(
+    "ignore",


order them alphabetically?

maelle · 2026-06-23T07:57:46Z

+    "median",
+    "concat"
+  )
+  known_codes <- c(0, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12)


add an explanatory comment?

Also, do we use explicit integers?

And would there be a way to define each code beside its name?

maelle · 2026-06-23T07:58:57Z

-      if (is.na(x)) {
+      idx <- pmatch(tolower(x), known_names)
+      if (is.na(idx)) {
+        if (!allow_rename && identical(tolower(x), "rename")) {


It'd be more logical to me to invert the two

if (identical(tolower(x), "rename") && !allow_rename) {

why identical btw?

maelle · 2026-06-23T07:59:41Z

+      if (is.na(idx)) {
+        if (!allow_rename && identical(tolower(x), "rename")) {
+          cli::cli_abort(
+            "{.val rename} is only supported by graph operators ({.fn union}, {.fn intersection}, {.fn compose}, {.fn disjoint_union}), not by this function."


Rephrase to

Can't use rename with function X (possible to add its name)

Hint: rename is only supported by blablabla

maelle · 2026-06-23T08:00:23Z

 #'
 #' The functions that support the combination of attributes have one or two
-#' extra arguments called `vertex.attr.comb` and/or `edge.attr.comb`
+#' extra arguments called `vertex_attr_combine` and/or `edge_attr_combine`


Seeing a clear name makes me so happy 😭 🪮

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

+#'       graph operators [union()], [intersection()], [compose()] and
+#'       [disjoint_union()] and preserves their historical behaviour.
+#'       Only those operators accept `"rename"`; [simplify()] and
+#'       [contract()] will reject it because the rename strategy has no


this sentence could be clarified: "it's hard to know where each attribute came from" or so?

maelle · 2026-06-23T08:03:04Z

      `as.undirected()` was deprecated in igraph 2.1.0.
      i Please use `as_undirected()` instead.
+      Warning:
+      The igraph option `edge.attr.comb` was deprecated in igraph 3.0.0.


maelle · 2026-06-23T08:04:21Z

+  u <- union(
+    gs$g1,
+    gs$g2,
+    vertex_attr_combine = list(weight = "sum", "rename")


add expectation on weight sum here too?

maelle · 2026-06-23T08:04:49Z

+
+test_that("union() picks first non-NA when only one input has the attr", {
+  gs <- make_named_pair()
+  u <- union(gs$g1, gs$g2, vertex_attr_combine = "first", byname = TRUE)


why byname and not by_name?

maelle · 2026-06-23T08:05:21Z

+
+  # Default option ("rename") preserves the historical suffixing behaviour.
+  u <- union(g1, g2)
+  expect_true(all(c("label_1", "label_2") %in% graph_attr_names(u)))


expect_all_true?

Add attribute combination support to graph operators (union, intersec…

c75e174

…tion, compose, disjoint_union)

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

krlmlr and others added 2 commits May 28, 2026 20:00

Merge branch 'main' into feat-attrib_comb

41c4b0c

make document work locally

5f7521a

krlmlr reviewed Jun 4, 2026

View reviewed changes

Comment thread tests/testthat/test-operators.R Outdated

krlmlr reviewed Jun 4, 2026

View reviewed changes

Comment thread R/operators.R Outdated

krlmlr reviewed Jun 4, 2026

View reviewed changes

schochastics and others added 3 commits June 6, 2026 20:20

Merge branch 'main' into feat-attrib_comb

cb49b91

chore: Auto-update from GitHub Actions

d7e8bdc

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

Merge branch 'main' into feat-attrib_comb

c26ab02

igraph deleted a comment from github-actions Bot Jun 10, 2026

schochastics and others added 3 commits June 10, 2026 12:27

chore: Auto-update from GitHub Actions

04ab715

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

krlmlr reviewed Jun 11, 2026

View reviewed changes

schochastics and others added 3 commits June 16, 2026 20:59

Merge remote-tracking branch 'origin/main' into feat-attrib_comb

5bebc2e

chore: Auto-update from GitHub Actions

05336ba

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

rename *_comb to *_combine

a85a78c

chore: Auto-update from GitHub Actions

d1ac089

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

maelle reviewed Jun 23, 2026

View reviewed changes

		"vertex_attr_comb" = list(name = "concat", "ignore"),
		"edge_attr_comb" = list(weight = "sum", name = "concat", "ignore"),

Uh oh!

Conversation

schochastics commented May 28, 2026

Uh oh!

schochastics commented May 28, 2026

Uh oh!

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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Jun 10, 2026

Uh oh!

github-actions Bot commented Jun 10, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Jun 16, 2026

Uh oh!

schochastics commented Jun 16, 2026

Uh oh!

github-actions Bot commented Jun 16, 2026

Uh oh!

maelle commented Jun 23, 2026

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants