Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Oct 26, 2025

Replaces manual vector recycling using rep(x, length.out = n) with vctrs::vec_recycle(x, n) for better error messages and consistency.

Changes

  • R/plot.R: Use vctrs::vec_size_common() to determine common size in igraph.Arrows(), then recycle all parameters to that size
  • R/games.R, R/centrality.R, R/epi.R: Replace rep(..., length.out = ) with vctrs::vec_recycle()
  • R/plot.shapes.R, R/plot.common.R, R/tkplot.R, R/sparsedf.R: Same pattern throughout plotting and utility functions

Example

Before:

recycle <- function(x) rep(x, length.out = n)
x1 <- recycle(x1)
y1 <- recycle(y1)

After:

n <- vctrs::vec_size_common(x1, y1, x2, y2)
x1 <- vctrs::vec_recycle(x1, n)
y1 <- vctrs::vec_recycle(y1, n)

This provides clearer error messages when inputs have incompatible sizes and centralizes recycling logic through a well-tested dependency already in use.

Original prompt

This section details on the original issue you should resolve

<issue_title>use vctrs in more places</issue_title>
<issue_description>when there is recycling

e.g. use vctrs::vec_size_common()</issue_description>

Comments on the Issue (you are @copilot in this section)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI changed the title [WIP] Use vctrs in more places for recycling Use vctrs for vector recycling Oct 26, 2025
Copilot AI requested a review from krlmlr October 26, 2025 19:17
Copilot finished work on behalf of krlmlr October 26, 2025 19:17
Copy link
Contributor

@krlmlr krlmlr left a 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 krlmlr removed their assignment Oct 26, 2025
@krlmlr krlmlr requested a review from maelle October 26, 2025 20:25
@maelle
Copy link
Contributor

maelle commented Nov 4, 2025

@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))
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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)
Copy link
Contributor

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

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.

use vctrs in more places

3 participants