feat: change constructors to use explicit arguments#2467
Conversation
Update constructor functions to accept explicit arguments instead of just `...`, matching their corresponding `make_*` and `sample_*` function signatures. This improves IDE autocomplete and makes the API clearer. Exceptions: - `directed_graph()` and `undirected_graph()` kept as `...` since they work with `make_()` where arguments can be passed in various orders - `tree()` and `from_literal()` kept as `...` with .lazy = TRUE due to their special lazy evaluation behavior Co-authored-by: krlmlr <1741643+krlmlr@users.noreply.github.com>
Co-authored-by: krlmlr <1741643+krlmlr@users.noreply.github.com>
R/make.R
Outdated
| mode = c("in", "out", "mutual", "undirected"), | ||
| center = 1 | ||
| ) { | ||
| constructor_spec(make_wheel, n, ..., mode = mode, center = center) |
There was a problem hiding this comment.
| constructor_spec(make_wheel, n, ..., mode = mode, center = center) | |
| check_dots_empty() | |
| constructor_spec(make_wheel, n, mode = mode, center = center) |
There was a problem hiding this comment.
Applied in c246631. Added check_dots_empty() to wheel() and other constructors with ... parameter (growing, bipartite_gnm, bipartite_gnp).
|
@copilot: Apply feedback, fix errors seen with |
- Add check_dots_empty() to constructors with ... parameter (wheel, growing, bipartite_gnm, bipartite_gnp) - Revert from_edgelist() and from_data_frame() to use ... for flexibility with graph_() - Fix documentation warnings by adding @inheritParams rlang::args_dots_empty - Remove @param ... from constructors that no longer have ... parameter - Update sample_() example to remove pattern that no longer works Co-authored-by: krlmlr <1741643+krlmlr@users.noreply.github.com>
Fixed in c246631. All devtools::check() errors and warnings resolved:
Status: 0 errors ✔ | 0 warnings ✔ | 0 notes ✔ Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
| #' @export | ||
| growing <- function(...) constructor_spec(sample_growing, ...) | ||
| growing <- function(n, m = 1, ..., directed = TRUE, citation = FALSE) { | ||
| check_dots_empty() |
There was a problem hiding this comment.
Add the check to the sampler as well.
There was a problem hiding this comment.
Added in 269d1bb. Added check_dots_empty() to sample_growing().
…uctors-to-use-explicit-arguments
- Merge main branch (includes version bump and modularity fix) - Add check_dots_empty() to sample_growing() as requested - Change all constructor_spec calls to use named arguments for consistency and clarity - This ensures better code readability and makes argument forwarding explicit All constructors now use named arguments when calling constructor_spec: - R/make.R: star, full_graph, ring, atlas, chordal_ring, circulant, line_graph, de_bruijn_graph, kautz_graph, from_prufer, bipartite_graph, full_bipartite_graph, full_multipartite, turan, full_citation_graph - R/games.R: gnp, gnm, growing, grg, smallworld, last_cit, cit_types, cit_cit_types, bipartite_gnm, bipartite_gnp, sbm, hierarchical_sbm, dot_product - R/adjacency.R: from_adjacency Co-authored-by: krlmlr <1741643+krlmlr@users.noreply.github.com>
Completed in 269d1bb:
All 40+ constructors now explicitly name all forwarded arguments for clarity and maintainability. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@schochastics: Good to merge? |
|
Do we even need to export the constructors? In #2500, there is a collision with |
I assume we cannot simple remove them since they were available before? I might have to think more about it but I currently dont see where these could be more handy than the corresponding make functions? |
Change constructors to use explicit arguments
Summary
Updated constructor functions to accept explicit arguments instead of just
..., matching their correspondingmake_*andsample_*function signatures. This improves IDE autocomplete and makes the API clearer.Latest changes
check_dots_empty()tosample_growing()- ensures consistency with the constructorconstructor_spec():constructor_spec(make_wheel, n = n, mode = mode, center = center)instead ofconstructor_spec(make_wheel, n, mode = mode, center = center)Testing
✅ All tests pass: 7,418 passed, 2 skipped (unrelated), 0 failures
✅
devtools::check()passes: 0 errors, 0 warnings, 0 notesOriginal prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.