-
-
Notifications
You must be signed in to change notification settings - Fork 206
Use vctrs for vector recycling
#2431
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: krlmlr <[email protected]>
vctrs for vector recycling
krlmlr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, but is this really complete?
|
@krlmlr for such patterns do we also want to add some linting to prevent future drift? |
|
|
||
| ## exo | ||
| exo <- cbind(rep(exo, length.out = vc)) | ||
| exo <- cbind(vctrs::vec_recycle(exo, vc)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this covered by a test? curious about this function actually works on a matrix.
| #' @cdocs igraph_hsbm_game | ||
| #' @cdocs igraph_hsbm_list_game | ||
| sample_hierarchical_sbm <- function(n, m, rho, C, p) { | ||
| # Determine sizes, treating non-lists as single elements |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
useless comment
| rholen <- if (is.list(rho)) length(rho) else 1 | ||
| Clen <- if (is.list(C)) length(C) else 1 | ||
|
|
||
| # Use vctrs to find common size, allowing recycling from length 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the comment is wrong, this doesn't use vctrs to find the common size?
| h.lty <- recycle(h.lty) | ||
| n <- vctrs::vec_size_common(x1, y1, x2, y2) | ||
|
|
||
| x1 <- vctrs::vec_recycle(x1, n) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a rare case where I'd find vertical alignment enjoyable
Replaces manual vector recycling using
rep(x, length.out = n)withvctrs::vec_recycle(x, n)for better error messages and consistency.Changes
vctrs::vec_size_common()to determine common size inigraph.Arrows(), then recycle all parameters to that sizerep(..., length.out = )withvctrs::vec_recycle()Example
Before:
After:
This provides clearer error messages when inputs have incompatible sizes and centralizes recycling logic through a well-tested dependency already in use.
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.