Skip to content

refactor: move optional arguments of games functions behind the ellipsis - #2760

Merged
krlmlr merged 3 commits into
mainfrom
claude/ellipsis-games
Jul 27, 2026
Merged

refactor: move optional arguments of games functions behind the ellipsis#2760
krlmlr merged 3 commits into
mainfrom
claude/ellipsis-games

Conversation

@krlmlr

@krlmlr krlmlr commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Part of the repo-wide ellipsis migration coordinated in #2757
see that PR for the full rationale (CONTRIBUTING.md, Argument Order and the Ellipsis).
Based directly on main (the registry split landed via #2779); a single topic commit.

What this does

Inserts ... between the defining arguments (head) and the optional
modifiers
(tail) of 36 exported games functions.
Arguments after ... become keyword-only.

  • Legacy positional or abbreviated calls are recovered by the generated
    ARG_HANDLE block and emit a single lifecycle::deprecate_soft("3.0.0", …)
    behavior is unchanged.
  • No defaults change, no arguments are renamed, deprecated functions untouched.
  • Registry: tools/migrations/games.R; blocks regenerated via
    Rscript tools/generate-migrations.R (idempotent, CI-checked).
  • Rd usage/arguments updated mechanically (see note below).

New signatures

function head (positional) keyword-only tail
asym_pref() nodes, types type.dist.matrix, pref.matrix, loops
cit_cit_types() n, edges, types pref, directed, attr
cit_types() n, edges, types pref, directed, attr
dot_product() vecs directed
gnm() n, m directed, loops
gnp() n, p directed, loops
grg() nodes, radius torus, coords
last_cit() n, edges agebins, pref, directed
pa() n, power, m out.dist, out.seq, out.pref, zero.appeal, directed, algorithm, start.graph
pa_age() n, pa.exp, aging.exp, m aging.bin, out.dist, out.seq, out.pref, directed, zero.deg.appeal, zero.age.appeal, deg.coef, age.coef, time.window
pref() nodes, types type.dist, fixed.sizes, pref.matrix, directed, loops
sample_asym_pref() nodes, types type.dist.matrix, pref.matrix, loops
sample_cit_cit_types() n, edges, types pref, directed, attr
sample_cit_types() n, edges, types pref, directed, attr
sample_correlated_gnp() old.graph, corr p, permutation
sample_degseq() out.deg, in.deg method
sample_dot_product() vecs directed
sample_fitness() no.of.edges, fitness.out, fitness.in loops, multiple
sample_fitness_pl() no.of.nodes, no.of.edges, exponent.out, exponent.in loops, multiple, finite.size.correction
sample_forestfire() nodes, fw.prob bw.factor, ambs, directed
sample_gnm() n, m directed, loops
sample_gnp() n, p directed, loops
sample_grg() nodes, radius torus, coords
sample_k_regular() no.of.nodes, k directed, multiple
sample_last_cit() n, edges agebins, pref, directed
sample_pa() n, power, m out.dist, out.seq, out.pref, zero.appeal, directed, algorithm, start.graph
sample_pa_age() n, pa.exp, aging.exp, m aging.bin, out.dist, out.seq, out.pref, directed, zero.deg.appeal, zero.age.appeal, deg.coef, age.coef, time.window
sample_pref() nodes, types type.dist, fixed.sizes, pref.matrix, directed, loops
sample_sbm() n, pref.matrix, block.sizes directed, loops
sample_smallworld() dim, size, nei, p loops, multiple
sample_traits() nodes, types, k type.dist, pref.matrix, directed
sample_traits_callaway() nodes, types edge.per.step, type.dist, pref.matrix, directed
sbm() n, pref.matrix, block.sizes directed, loops
smallworld() dim, size, nei, p loops, multiple
traits() nodes, types, k type.dist, pref.matrix, directed
traits_callaway() nodes, types edge.per.step, type.dist, pref.matrix, directed

Notes for review

  • The environment used to prepare this PR cannot install igraph.r2cdocs
    (GitHub API unreachable), so man/*.Rd files were updated mechanically
    (usage + ... argument item) instead of via devtools::document().
    R CMD check's usage↔formals validation passes; a follow-up
    devtools::document() run may reflow whitespace but should produce no
    semantic diff.
  • Package tests that called these functions positionally were updated to
    named arguments (they are the same soft-deprecation user code will see).

@krlmlr
krlmlr force-pushed the claude/ellipsis-games branch 2 times, most recently from 8513c90 to 6b344a7 Compare July 26, 2026 11:09
@github-actions

Copy link
Copy Markdown
Contributor

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

  • ❗🐌as_adjacency_matrix: 747ms -> 754ms [+0.13%, +1.7%]
  • ✔️as_biadjacency_matrix: 735ms -> 738ms [-1.68%, +2.56%]
  • ✔️as_data_frame_both: 1.51ms -> 1.5ms [-3.07%, +1.99%]
  • ✔️as_long_data_frame: 3.96ms -> 3.94ms [-3.34%, +2.6%]
  • ✔️es_attr_filter: 2.66ms -> 2.67ms [-1.57%, +2.83%]
  • ✔️graph_from_adjacency_matrix: 114ms -> 114ms [-1.76%, +1.58%]
  • ✔️graph_from_data_frame: 3.32ms -> 3.28ms [-2.82%, +0.11%]
  • ✔️vs_attr_filter: 1.52ms -> 1.51ms [-3.48%, +1.72%]
  • ✔️vs_by_name: 958µs -> 956µs [-2.09%, +1.61%]
    Further explanation regarding interpretation and methodology can be found in the documentation.

@krlmlr
krlmlr marked this pull request as ready for review July 26, 2026 15:03
@krlmlr
krlmlr force-pushed the claude/ellipsis-games branch 2 times, most recently from a97ad56 to 782b681 Compare July 26, 2026 15:20
@github-actions

Copy link
Copy Markdown
Contributor

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

  • ✔️as_adjacency_matrix: 801ms -> 804ms [-0.54%, +1.13%]
  • ✔️as_biadjacency_matrix: 796ms -> 790ms [-2.4%, +0.94%]
  • ✔️as_data_frame_both: 1.58ms -> 1.56ms [-2.78%, +0.91%]
  • ✔️as_long_data_frame: 4.1ms -> 4.04ms [-4.17%, +1.1%]
  • ✔️es_attr_filter: 2.86ms -> 2.82ms [-3.87%, +0.76%]
  • ✔️graph_from_adjacency_matrix: 125ms -> 124ms [-2.18%, +0.84%]
  • ✔️graph_from_data_frame: 3.46ms -> 3.52ms [-0.88%, +4.59%]
  • ✔️vs_attr_filter: 1.66ms -> 1.65ms [-3.44%, +1.73%]
  • ✔️vs_by_name: 1.06ms -> 1.06ms [-2.29%, +3.17%]
    Further explanation regarding interpretation and methodology can be found in the documentation.

Insert `...` between the defining arguments and the optional
modifiers of 36 functions,
following the zoning rules in CONTRIBUTING.md.
Legacy positional and abbreviated calls are recovered
by the generated ARG_HANDLE blocks
(registry: tools/migrations/games.R)
and emit a single soft deprecation for igraph 3.0.0.
No defaults change and no arguments are renamed.

Functions: asym_pref, cit_cit_types, cit_types, dot_product, gnm, gnp, grg, last_cit, pa, pa_age, pref, sample_asym_pref, sample_cit_cit_types, sample_cit_types, sample_correlated_gnp, sample_degseq, sample_dot_product, sample_fitness, sample_fitness_pl, sample_forestfire, sample_gnm, sample_gnp, sample_grg, sample_k_regular, sample_last_cit, sample_pa, sample_pa_age, sample_pref, sample_sbm, sample_smallworld, sample_traits, sample_traits_callaway, sbm, smallworld, traits, traits_callaway

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016M32izVHZPfxAqemAe4BrX
@krlmlr
krlmlr force-pushed the claude/ellipsis-games branch from 782b681 to 45c0e12 Compare July 26, 2026 16:23
claude added 2 commits July 26, 2026 19:19
Same fix as #2781: the migration rewriter collapsed multi-line enum
defaults and air skips c() calls, so restore the original layout.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016M32izVHZPfxAqemAe4BrX
Add argument-coverage tests for the 36 migrated games functions:
every keyword-only tail argument is passed by name with a real assertion,
and every function's legacy positional call path
is checked to warn and recover.
Spec twins (`pa()`, `gnp()`, ...) are covered
by compact `sample_()` equivalence tests.

Known limitations exercised and documented in the tests:
`sample_pa_age()`'s `out.seq`/`out.dist` cannot produce a graph
(pre-existing, also broken on main),
and `sample_last_cit()`'s recovery passes `pref` alongside `agebins`
because the shim forces interdependent defaults before recovery.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016M32izVHZPfxAqemAe4BrX
@github-actions

Copy link
Copy Markdown
Contributor

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

  • ✔️as_adjacency_matrix: 591ms -> 590ms [-1.58%, +1.39%]
  • ✔️as_biadjacency_matrix: 566ms -> 569ms [-0.47%, +1.59%]
  • ✔️as_data_frame_both: 1.24ms -> 1.23ms [-2.65%, +0.82%]
  • ✔️as_long_data_frame: 3.28ms -> 3.31ms [-1.84%, +3.37%]
  • ✔️es_attr_filter: 2.33ms -> 2.35ms [-1.66%, +3.04%]
  • ✔️graph_from_adjacency_matrix: 97.6ms -> 99.4ms [-3%, +6.67%]
  • ✔️graph_from_data_frame: 2.77ms -> 2.77ms [-1.02%, +1.4%]
  • ✔️vs_attr_filter: 1.22ms -> 1.23ms [-3.31%, +5.21%]
  • ✔️vs_by_name: 781µs -> 778µs [-1.6%, +0.82%]
    Further explanation regarding interpretation and methodology can be found in the documentation.

@krlmlr
krlmlr merged commit 53227ec into main Jul 27, 2026
9 checks passed
@krlmlr
krlmlr deleted the claude/ellipsis-games branch July 27, 2026 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants