feat: add tooling for parameter migration by schochastics · Pull Request #2684 · igraph/rigraph

schochastics · 2026-06-01T14:36:12Z

@krlmlr a suggestion derived from the discussions in #2677. This would be a way to generate arg migration functions.

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

krlmlr

Nice, thanks for driving this!

Rework the migration tooling per review on #2684. - Registry entries now declare `old`/`new` as literal function signatures; renames are inferred from bare-symbol defaults (`c = c_renamed`) and defaults are read off the `new` formals. Less typing, no parallel renames/defaults bookkeeping. - Generated helpers now recover legacy calls by position *and* by (partial) name: a renamed-away old name (`attr =`) or an abbreviation of a new arg (`weig =`) is matched via charmatch(); ambiguous prefixes error. - Deprecation message shows the detected vs. requested signature side by side and drops the "pre-3.0.0" phrasing; build it with paste0(). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Replace the standalone check-migrations workflow with two cheaper hooks: - A testthat helper regenerates R/handle-args-*.R whenever tools/migrations.R (or the generator) is newer, so editing the registry and running the tests refreshes the helpers automatically. - The rcc smoke job regenerates from the source checkout and fails on any resulting git diff, before the auto-commit steps. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

- Reframe "removed/reordered args" as a supported workaround: place the affected argument after `...` so it is recovered by name, not position. - Explain why `.user_env` is threaded: deprecate_soft() gates on is_direct(user_env) before the verbosity override, so the helper's extra call frame would otherwise suppress the warning. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

krlmlr

Beautiful!

krlmlr · 2026-06-02T06:53:56Z

And if we think that a little further: what if our code generator patches the code in place, delineated by # BEGIN ARG_HANDLE and # END ARG_HANDLE comments? No handler function, no magic, no caller env crap.

schochastics · 2026-06-02T07:29:49Z

And if we think that a little further: what if our code generator patches the code in place, delineated by # BEGIN ARG_HANDLE and # END ARG_HANDLE comments? No handler function, no magic, no caller env crap.

Interesting idea, I will iterate a bit more

Per review on #2684, pivot from generated handler functions to in-place codegen. The generator now rewrites the code between `# BEGIN/END GENERATED ARG_HANDLE` markers inside each migrated function body instead of emitting a standalone `handle_args_<fn>()`. - The recovery runs inline: an immediately-invoked function does the pure matching (temps stay scoped, no environment access), the host frame reassigns its own locals, and a single `lifecycle::deprecate_soft()` is emitted directly in the function. Its default `user_env` (caller_env(2)) resolves to the user, so the `.user_env` parameter and all the env threading are gone. - Output is laid out exactly as `air` formats it, so the host files stay clean (no air.toml exclusion needed; the old one is dropped). - Registry: document that old-side default values are ignored (default changes live in `new`); note the bare-symbol-default edge. - Fixture moves to R/migration-fixture.R (a marker-carrying function); the old R/handle-args-migration_fixture.R is removed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Move the regenerate + `git diff` drift guard out of the rcc smoke job into the `custom/after-install` composite action (R is available there, and it is the natural place per review). The testthat helper still regenerates locally when the registry is newer. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

schochastics · 2026-06-02T16:43:54Z

Iterated one more time

krlmlr

Sweet! We're getting there. I really like the new inline checkers, really looking forward to rolling it out.

I wonder if we could also get a series of snapshot tests that exercise the migration fixture with lifecycle warnings enabled.

Address review on #2684: - Drop the inline IIFE in the generated ARG_HANDLE block in favour of a plain, named, hand-written helper `migrate_recover_args()` (R/migrate-args.R) that takes the per-function maps and returns the recovered values plus the deprecation message parts, or NULL. It is pure (no environment access) and easy to step through in a debugger. The generated block now just supplies the configuration, assigns the results, and emits the inline deprecate_soft(). - Collapse the three-line generated header comment onto the BEGIN marker, which may now carry a trailing note; the generator's marker regex captures the function name up to the first non-identifier character. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

schochastics · 2026-06-02T19:59:05Z

Switched the inline closure for a plain named helper migrate_recover_args() in R/migrate-args.R. The generated block just passes config and assigns results, with deprecate_soft() still inline. Also collapsed the generated comment onto the marker. Left the after-install drift check for now

krlmlr

Wonderful!

I'd still like to see a test-migration-fixture.R with many snapshot tests. May need to extend the arg names to also cover the "partial matching" case.

Alternatively (or in addition), we can start applying this to a few functions.

Address review on #2684: - Wrap the generated ARG_HANDLE block in `if (...length() > 0L)` so a correct new-API call (nothing in `...`) skips the `migrate_recover_args()` call entirely; the now-redundant `if (!is.null())` check is dropped. - Extend the test fixture to `migration_fixture(graph, n, weight = weights, type, directed)` so it exercises a rename (`weight -> weights`), unique abbreviations (`ty`, `dir`) and an ambiguous one (`weig` matches both `weight` and `weights`). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Rename test-handle-args.R to test-migration-fixture.R and add snapshot tests (run under lifecycle_verbosity = "warning") covering positional recovery, the renamed-away old name, unique/ambiguous abbreviations, and the unknown / conflict / too-many error messages, alongside the migrate_recover_args() unit tests and the generator in-sync checks. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Add a fixture case where a positional value and a named abbreviation are supplied together, pinning down the interleaved recovery and the positional-then-named ordering in the "Detected call" message. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

schochastics · 2026-06-03T06:07:29Z

If this is ok we could merge to main and apply in #2677?

krlmlr

Thanks, great! Would you like to do a follow-up PR for refining this?

krlmlr · 2026-06-03T07:15:26Z

+  # matches both `weight` and `weights`). Consumed by
+  # tests/testthat/test-migration-fixture.R.
+  migration_fixture = list(
+    old = function(graph, n, weight = weights, type, directed) {},


Playing around the following axis would be great in a follow-up PR, including calls with g = ... or gr = ... instead of graph = ...:

Suggested change

old = function(graph, n, weight = weights, type, directed) {},

old = function(graph, n, weight = weights, kind = type, directed) {},

schochastics and others added 2 commits June 1, 2026 16:31

add arg migration

892e10f

chore: Auto-update from GitHub Actions

e83a58d

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

krlmlr reviewed Jun 1, 2026

View reviewed changes

Comment thread tools/README.md Outdated

krlmlr reviewed Jun 1, 2026

View reviewed changes

schochastics and others added 3 commits June 2, 2026 07:24

krlmlr reviewed Jun 2, 2026

View reviewed changes

Comment thread .github/workflows/R-CMD-check.yaml Outdated

Comment thread tools/migrations.R Outdated

Comment thread tools/README.md Outdated

Comment thread tools/README.md Outdated

schochastics and others added 3 commits June 2, 2026 18:39

docs: describe in-place ARG_HANDLE blocks; drop .user_env rationale

3e6c9ea

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

krlmlr reviewed Jun 2, 2026

View reviewed changes

Comment thread .github/workflows/custom/after-install/action.yml

krlmlr reviewed Jun 2, 2026

View reviewed changes

Comment thread R/migration-fixture.R Outdated

Comment thread R/migration-fixture.R Outdated

krlmlr reviewed Jun 2, 2026

View reviewed changes

Comment thread R/migration-fixture.R Outdated

schochastics and others added 3 commits June 3, 2026 07:53

schochastics marked this pull request as ready for review June 3, 2026 06:32

krlmlr approved these changes Jun 3, 2026

View reviewed changes

krlmlr merged commit 49d18ae into main Jun 3, 2026
6 checks passed

krlmlr deleted the feat/migration-helper-codegen branch June 3, 2026 17:36

schochastics mentioned this pull request Jun 3, 2026

fix: harden air support and broaden migration fixture to the full arg-matching matrix #2686

Open

	old = function(graph, n, weight = weights, type, directed) {},
	old = function(graph, n, weight = weights, kind = type, directed) {},

Uh oh!

Conversation

schochastics commented Jun 1, 2026

Uh oh!

Uh oh!

krlmlr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

krlmlr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

krlmlr commented Jun 2, 2026

Uh oh!

schochastics commented Jun 2, 2026

Uh oh!

schochastics commented Jun 2, 2026

Uh oh!

Uh oh!

krlmlr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

schochastics commented Jun 2, 2026

Uh oh!

krlmlr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

schochastics commented Jun 3, 2026

Uh oh!

krlmlr left a comment

Choose a reason for hiding this comment

Uh oh!

krlmlr Jun 3, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants