-
Notifications
You must be signed in to change notification settings - Fork 56
add vctrs methods to make rectangling nicer #226
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
Conversation
| }) | ||
|
|
||
|
|
||
| test_that("gh_response objects can be combined via vctrs #161", { |
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.
👍
| # Add vctrs methods that strip attributes from gh_response when combining, | ||
| # enabling rectangling via unnesting etc | ||
| # See <https://github.com/r-lib/gh/issues/161> for more details | ||
| #' @exportS3Method vctrs::vec_ptype2 |
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.
Here and below: exportS3Method uses delayed method registration. I think we need a regular export here: @export.
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.
Got it, thanks!
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.
@lionel- I'm getting an error here with a normal @export because we don't import the generic vec_ptype2 (or maybe because we don't export it?). I think delayed might be better (and make vctrs a suggests)?
https://github.com/r-lib/gh/actions/runs/17863952750/job/50801557570?pr=226#step:6:181
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.
oh you're right!
lionel-
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.
Could you please revert the air formatting commit? PRs should be focused on one thing so they are easier to evaluate by the reviewer. An air format commit should be its own PR.
15ca523 to
3a6d520
Compare
4595042 to
323affe
Compare
|
Thank you! |
Closes #161, resurrects #201 (got closed accidentally) as part of tidy dev day:
Adds vctrs methods provided by @DavisVaughan to help
gh_responseplay more nicely with vctrs rectangling toolkit.Test instructions: