feat: Check that exactly one vertex is passed #2556
Conversation
There was a problem hiding this comment.
Pull request overview
Tightens argument validation for parameters that are expected to identify a single vertex, ensuring exactly one vertex is provided (instead of allowing 0 or >1 and relying on downstream behavior).
Changes:
- Update the Stimulus
VERTEX/VERTEX_ROOTtype conversions to requirelength(...) == 1and adjust corresponding error text. - Regenerate/update auto-generated R interface code to apply the stricter checks across affected
_implwrappers. - Minor YAML literal formatting change for
CLOSURE(|→|-), reflected in generated output formatting.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tools/stimulus/types-RR.yaml | Updates vertex argument conversion templates to enforce exactly-one-vertex semantics and align error messages. |
| R/aaa-auto.R | Applies regenerated argument checks and updated error messages across many internal _impl functions. |
| if (length(source) != 1) { | ||
| cli::cli_abort( | ||
| "{.arg source} must specify at least one vertex", | ||
| "{.arg source} must specify exactly one vertex", | ||
| call = rlang::caller_env() | ||
| ) |
There was a problem hiding this comment.
Same as above: updating to “must specify exactly one vertex” will require updating snapshot tests for st_min_cuts()/all_st_mincuts_impl (see tests/testthat/_snaps/flow.md) so CI doesn’t fail on stale snapshots.
| %I% <- as_igraph_vs(%I1%, %I%) | ||
| if (length(%I%) == 0) { | ||
| if (length(%I%) != 1) { | ||
| cli::cli_abort( | ||
| "{.arg %I%} must specify at least one vertex", | ||
| "{.arg %I%} must specify exactly one vertex", | ||
| call = rlang::caller_env() | ||
| ) |
There was a problem hiding this comment.
This change tightens the VERTEX conversion to error when more than one vertex is supplied. There are tests for several affected user-facing wrappers (e.g., are_adjacent(), random_walk(), st_cuts()), but none appear to assert the new “multiple vertices” error path. Adding a small test to cover the >1-length case would help prevent regressions in this stricter input validation.
| if (length(source) != 1) { | ||
| cli::cli_abort( | ||
| "{.arg source} must specify at least one vertex", | ||
| "{.arg source} must specify exactly one vertex", | ||
| call = rlang::caller_env() | ||
| ) |
There was a problem hiding this comment.
The error message for invalid source/target changed from “must specify at least one vertex” to “must specify exactly one vertex”. This will break existing snapshot expectations (e.g., tests/testthat/_snaps/flow.md for st_cuts()), so the corresponding snapshots/tests should be updated to match the new wording/behavior.
May trigger revdepcheck failures.