Skip to content

refactor: snake_case attr-comb options & arguments, soft-deprecate dotted names#2742

Open
schochastics wants to merge 2 commits into
mainfrom
par-attr-comb-options
Open

refactor: snake_case attr-comb options & arguments, soft-deprecate dotted names#2742
schochastics wants to merge 2 commits into
mainfrom
par-attr-comb-options

Conversation

@schochastics

Copy link
Copy Markdown
Contributor

Split out from #2676 per review feedback ("I'd like to review the par code in a separate PR"). #2676 now keeps the original dotted option/argument names; this PR carries the naming refactor on its own so it can be reviewed in isolation.

What this does

  • igraph options: rename vertex.attr.combvertex_attr_combine, edge.attr.combedge_attr_combine, and the new graph.attr.combgraph_attr_combine. Reading/setting the old dotted keys still works via back-compatible aliasing in igraph_opt() / igraph_options(), emitting a single lifecycle::deprecate_soft() warning (R/par.R).
  • Function arguments: simplify(), as_undirected() and contract() gain snake_case edge_attr_combine / vertex_attr_combine arguments via the argument-migration infrastructure (tools/migrations.R); the dotted names keep working and soft-deprecate.
  • Graph operators: union() / intersection() / compose() / disjoint_union() use the snake_case *_combine argument names.
  • Updates the stimulus codegen defaults (types-RR.yaml), regenerated *_impl defaults, and the affected tests/snapshots.

Base branch

Based on feat-attrib_comb (not main) so the diff shows only the renaming. Once #2676 merges, retarget this to main and the diff collapses to the par changes alone.

🤖 Generated with Claude Code

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

This is how benchmark results would change (along with a 95% confidence interval in relative change) if 1453158 is merged into feat-attrib_comb:

  • ✔️as_adjacency_matrix: 706ms -> 701ms [-2.32%, +0.95%]
  • ✔️as_biadjacency_matrix: 734ms -> 736ms [-1.27%, +1.75%]
  • ✔️as_data_frame_both: 1.48ms -> 1.56ms [-8.29%, +19.78%]
  • ✔️as_long_data_frame: 3.85ms -> 3.91ms [-1.87%, +5.16%]
  • ✔️es_attr_filter: 2.84ms -> 2.88ms [-1.61%, +4.71%]
  • ✔️graph_from_adjacency_matrix: 147ms -> 148ms [-1.36%, +2.48%]
  • ✔️graph_from_data_frame: 3.38ms -> 3.44ms [-1.16%, +4.6%]
  • ✔️vs_attr_filter: 1.54ms -> 1.56ms [-3.55%, +5.83%]
  • ✔️vs_by_name: 983µs -> 983µs [-3.85%, +3.81%]
    Further explanation regarding interpretation and methodology can be found in the documentation.

Base automatically changed from feat-attrib_comb to main July 9, 2026 08:42
… names

Split out from #2676 per review feedback: the renaming of the igraph
options and the *.attr.comb function arguments (with soft-deprecation of
the dotted names) is carried here so it can be reviewed on its own.

- igraph options vertex.attr.comb / edge.attr.comb (and the new
  graph.attr.comb) are renamed to snake_case *_combine, with
  back-compatible aliasing in igraph_opt() / igraph_options(); the dotted
  keys still read and set, soft-deprecated (R/par.R).
- simplify() / as_undirected() / contract() gain snake_case
  edge_attr_combine / vertex_attr_combine arguments via the
  argument-migration infrastructure (tools/migrations.R); dotted names
  keep working and soft-deprecate.
- union() / intersection() / compose() / disjoint_union() use the
  snake_case *_combine argument names.
- Updates stimulus codegen defaults (types-RR.yaml), regenerated *_impl
  defaults, and the affected tests/snapshots.

Retains the igraph_i_options() ...length() fix from the base branch.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

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

  • ✔️as_adjacency_matrix: 924ms -> 934ms [-1.4%, +3.52%]
  • ✔️as_biadjacency_matrix: 899ms -> 899ms [-1.57%, +1.56%]
  • ✔️as_data_frame_both: 1.68ms -> 1.67ms [-3.86%, +2.57%]
  • ✔️as_long_data_frame: 4.38ms -> 4.22ms [-13.07%, +5.49%]
  • ✔️es_attr_filter: 3.16ms -> 3.09ms [-5.92%, +1.93%]
  • ✔️graph_from_adjacency_matrix: 164ms -> 168ms [-1.87%, +6.71%]
  • ❗🐌graph_from_data_frame: 3.63ms -> 3.77ms [+1.29%, +6.67%]
  • ✔️vs_attr_filter: 1.84ms -> 1.81ms [-7.05%, +3.86%]
  • ✔️vs_by_name: 1.11ms -> 1.11ms [-4.04%, +3.9%]
    Further explanation regarding interpretation and methodology can be found in the documentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant