Skip to content

refactor: move optional arguments of make functions behind the ellipsis#2761

Open
krlmlr wants to merge 2 commits into
mainfrom
claude/ellipsis-make
Open

refactor: move optional arguments of make functions behind the ellipsis#2761
krlmlr wants to merge 2 commits into
mainfrom
claude/ellipsis-make

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 23 exported make 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/make.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
bipartite_graph() types, edges directed
chordal_ring() n, w directed
circulant() n, shifts directed
empty_graph() n directed
full_bipartite_graph() n1, n2 directed, mode
full_citation_graph() n directed
full_graph() n directed, loops
full_multipartite() n directed, mode
make_bipartite_graph() types, edges directed
make_chordal_ring() n, w directed
make_circulant() n, shifts directed
make_empty_graph() n directed
make_full_bipartite_graph() n1, n2 directed, mode
make_full_citation_graph() n directed
make_full_graph() n directed, loops
make_full_multipartite() n directed, mode
make_ring() n directed, mutual, circular
make_star() n mode, center
make_tree() n, children mode
realize_degseq() out.deg, in.deg allowed.edge.types, method
ring() n directed, mutual, circular
sample_tree() n directed, method
star() n mode, center

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-make branch 2 times, most recently from 2798838 to 7e520e0 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 7e520e0 is merged into main:

  • ✔️as_adjacency_matrix: 761ms -> 760ms [-0.71%, +0.4%]
  • ✔️as_biadjacency_matrix: 740ms -> 737ms [-0.98%, +0.15%]
  • ✔️as_data_frame_both: 1.48ms -> 1.47ms [-2.61%, +1.31%]
  • ✔️as_long_data_frame: 3.94ms -> 3.9ms [-3.48%, +1.82%]
  • ✔️es_attr_filter: 2.67ms -> 2.68ms [-0.79%, +1.6%]
  • ✔️graph_from_adjacency_matrix: 116ms -> 116ms [-0.7%, +0.78%]
  • ✔️graph_from_data_frame: 3.34ms -> 3.35ms [-1.09%, +1.89%]
  • ✔️vs_attr_filter: 1.53ms -> 1.52ms [-3.76%, +1.84%]
  • ✔️vs_by_name: 963µs -> 966µs [-0.9%, +1.66%]
    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
Insert `...` between the defining arguments and the optional
modifiers of 23 functions,
following the zoning rules in CONTRIBUTING.md.
Legacy positional and abbreviated calls are recovered
by the generated ARG_HANDLE blocks
(registry: tools/migrations/make.R)
and emit a single soft deprecation for igraph 3.0.0.
No defaults change and no arguments are renamed.

Functions: bipartite_graph, chordal_ring, circulant, empty_graph, full_bipartite_graph, full_citation_graph, full_graph, full_multipartite, make_bipartite_graph, make_chordal_ring, make_circulant, make_empty_graph, make_full_bipartite_graph, make_full_citation_graph, make_full_graph, make_full_multipartite, make_ring, make_star, make_tree, realize_degseq, ring, sample_tree, star

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016M32izVHZPfxAqemAe4BrX
@krlmlr
krlmlr force-pushed the claude/ellipsis-make branch from 7e520e0 to 7a52ea6 Compare July 26, 2026 16:23
Add argument-coverage tests for the 23 migrated constructors in R/make.R:
every keyword-only tail argument is passed by name with a non-default value,
and the legacy positional form is checked to still work with a deprecation.
Constructor-spec twins get one make_() equivalence test with named tail args.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016M32izVHZPfxAqemAe4BrX
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