Skip to content

Assorted fixes#454

Open
VisruthSK wants to merge 8 commits into
masterfrom
assorted-changes
Open

Assorted fixes#454
VisruthSK wants to merge 8 commits into
masterfrom
assorted-changes

Conversation

@VisruthSK

@VisruthSK VisruthSK commented Jun 18, 2026

Copy link
Copy Markdown
Member

This PR was made with assistance from Codex.

Summary

Fix failures in R-devel due to changes to base apply(). Bumped roxygen to 8.0.0 and regenerated documentation. Also normed files to lf ending.

Copyright and Licensing

By submitting this pull request, the copyright holder is agreeing to license the submitted work under the following licenses:

@github-actions

Copy link
Copy Markdown

This is how benchmark results would change (along with a 95% confidence interval in relative change) if 3f6b50d is merged into master:

  • ✔️as_draws_array: 140ms -> 139ms [-2.11%, +0.51%]
  • ✔️as_draws_df: 65.8ms -> 65.7ms [-0.72%, +0.63%]
  • ✔️as_draws_list: 152ms -> 150ms [-2.7%, +0.08%]
  • ✔️as_draws_matrix: 24.3ms -> 24.4ms [-0.95%, +1.72%]
  • ✔️as_draws_rvars: 112ms -> 112ms [-0.58%, +1.35%]
  • 🚀summarise_draws_100_variables: 730ms -> 727ms [-0.77%, -0.01%]
  • ✔️summarise_draws_10_variables: 81.5ms -> 81.6ms [-0.2%, +0.5%]
    Further explanation regarding interpretation and methodology can be found in the documentation.

@github-actions

Copy link
Copy Markdown

This is how benchmark results would change (along with a 95% confidence interval in relative change) if 93af05f is merged into master:

  • 🚀as_draws_array: 145ms -> 143ms [-2.28%, -0.37%]
  • ✔️as_draws_df: 69.7ms -> 69.6ms [-1.57%, +1.32%]
  • ✔️as_draws_list: 158ms -> 159ms [-0.92%, +1.5%]
  • ✔️as_draws_matrix: 24.7ms -> 24.9ms [-0.35%, +1.87%]
  • ✔️as_draws_rvars: 116ms -> 115ms [-1.8%, +0.28%]
  • ✔️summarise_draws_100_variables: 733ms -> 734ms [-0.33%, +0.48%]
  • ✔️summarise_draws_10_variables: 82.1ms -> 82.3ms [-0.25%, +0.58%]
    Further explanation regarding interpretation and methodology can be found in the documentation.

@github-actions

Copy link
Copy Markdown

This is how benchmark results would change (along with a 95% confidence interval in relative change) if 59398ee is merged into master:

  • ✔️as_draws_array: 130ms -> 129ms [-1.64%, +0.5%]
  • ✔️as_draws_df: 71.7ms -> 71.3ms [-2.06%, +0.91%]
  • ✔️as_draws_list: 144ms -> 142ms [-3.02%, +0.83%]
  • ✔️as_draws_matrix: 20ms -> 19.9ms [-1.51%, +0.17%]
  • ✔️as_draws_rvars: 106ms -> 106ms [-0.96%, +2.04%]
  • ✔️summarise_draws_100_variables: 600ms -> 601ms [-0.19%, +0.25%]
  • ✔️summarise_draws_10_variables: 67.4ms -> 67.4ms [-0.38%, +0.33%]
    Further explanation regarding interpretation and methodology can be found in the documentation.

@VisruthSK VisruthSK marked this pull request as ready for review June 18, 2026 06:12
@VisruthSK VisruthSK requested a review from paul-buerkner June 18, 2026 06:13
@github-actions

Copy link
Copy Markdown

This is how benchmark results would change (along with a 95% confidence interval in relative change) if bc7f533 is merged into master:

  • ✔️as_draws_array: 138ms -> 137ms [-1.44%, +0.04%]
  • ✔️as_draws_df: 77ms -> 77.7ms [-1.91%, +3.73%]
  • ✔️as_draws_list: 153ms -> 153ms [-0.41%, +0.79%]
  • ✔️as_draws_matrix: 24.2ms -> 24ms [-1.9%, +0.25%]
  • ✔️as_draws_rvars: 129ms -> 128ms [-1.87%, +0.92%]
  • ✔️summarise_draws_100_variables: 825ms -> 825ms [-2.65%, +2.79%]
  • ✔️summarise_draws_10_variables: 96.3ms -> 95.8ms [-2.06%, +1.08%]
    Further explanation regarding interpretation and methodology can be found in the documentation.

@paul-buerkner

Copy link
Copy Markdown
Collaborator

thank you! I have been in contact with the CRAN team and it may be that the commit that causes posterior to fail on R-devel may be reverted. But perhaps it would make them happy if posterior would still pass already now. And it may make posterior more robust against these changes in the future.

Do you expect any negative "side effects" of this PR? I.e. some aspects of rvar computation becoming more prone to bugs or errors? If not, I am happy to merge this PR.

@paul-buerkner

Copy link
Copy Markdown
Collaborator

@mjskay do you have a few minutes to review this PR? You can judge rvar code better than I can.

@paul-buerkner paul-buerkner requested a review from mjskay June 18, 2026 09:59
@VisruthSK

Copy link
Copy Markdown
Member Author

I'm realizing that it would've been better to separate out the rvar changes from the other ones in this PR, but hopefully that doesn't make it too difficult to review.

I don't think there should be any major correctness or performance changes--I didn't check but I can't imagine too many people were relying on the vectrs default behaviours for as.array() and as.matrix(). I guess a rev dep check might be useful? But also might be overkill...

Also would like to get @mjskay to review if possible before merging.

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.

2 participants