Skip to content

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

Open
schochastics wants to merge 1 commit into
mainfrom
par-attr-comb-options
Open

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

Conversation

@schochastics

@schochastics schochastics commented Jul 8, 2026

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 kept 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 (registry entries in tools/migrations/attr-comb.R, spliced by Rscript tools/generate-migrations.R; schema in tools/migrations/README.md); 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.
  • Points the stale tools/migrations.R schema references in sibling registry files and helper comments at tools/migrations/README.md, where the schema documentation lives since the registry split (refactor(migrations): split the registry into per-topic files #2779).

Base branch

#2676 has merged, so this PR is based on main and rebased into a single commit on top of it; the attr-comb migration entries live in their own registry file tools/migrations/attr-comb.R so that the open per-topic migration PRs can append to conversion.R, community.R, etc. without conflicting here.

@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
@schochastics
schochastics force-pushed the par-attr-comb-options branch from 1453158 to 2f878f3 Compare July 15, 2026 19:10
@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.

@github-actions

Copy link
Copy Markdown
Contributor

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

  • ✔️as_adjacency_matrix: 760ms -> 758ms [-0.87%, +0.2%]
  • ✔️as_biadjacency_matrix: 742ms -> 740ms [-0.98%, +0.45%]
  • ✔️as_data_frame_both: 1.46ms -> 1.47ms [-1.02%, +1.2%]
  • ✔️as_long_data_frame: 3.87ms -> 3.87ms [-0.66%, +1.04%]
  • ✔️es_attr_filter: 2.77ms -> 2.75ms [-2.88%, +1.57%]
  • ✔️graph_from_adjacency_matrix: 118ms -> 117ms [-1.93%, +1.58%]
  • ❗🐌graph_from_data_frame: 3.32ms -> 3.38ms [+1.01%, +2.53%]
  • ✔️vs_attr_filter: 1.5ms -> 1.5ms [-1.86%, +0.59%]
  • ✔️vs_by_name: 1ms -> 987µs [-4.57%, +1.83%]
    Further explanation regarding interpretation and methodology can be found in the documentation.

…tted names

Rename the attribute-combination options and arguments to snake_case:
`edge.attr.comb` -> `edge_attr_combine` and
`vertex.attr.comb` -> `vertex_attr_combine`,
both as igraph options and as arguments to `simplify()`,
`as_undirected()`, and `contract()`.
The dotted argument names keep working behind `...`
under a single soft-deprecation,
via generated ARG_HANDLE blocks
driven by the new tools/migrations/attr-comb.R registry file.
Dotted option names are soft-deprecated with a warning shim.

Also point the stale schema references in sibling registry files
and helper comments at tools/migrations/README.md,
where the schema documentation lives
since the registry was split into per-topic files.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016M32izVHZPfxAqemAe4BrX
@krlmlr
krlmlr force-pushed the par-attr-comb-options branch from ba22989 to c5de7bb Compare July 26, 2026 19:29
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.

2 participants