Skip to content
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

Implement pivot_longer() #204

Merged
merged 14 commits into from
Mar 8, 2021
Merged

Conversation

markfairbanks
Copy link
Collaborator

@markfairbanks markfairbanks commented Mar 1, 2021

A few notes:

  • I added vec_cast() to add_dt_wrappers() - I wasn't sure if there was a different way you'd rather add that.
  • I implemented a version of pmap_chr() taken from rlang's compat-purrr.R file for use in str_extract() to avoid a purrr dependency.

Copy link
Member

@hadley hadley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thanks! A few minor comments/questions below.

R/step-call-pivot_longer.R Outdated Show resolved Hide resolved
R/step-call-pivot_longer.R Outdated Show resolved Hide resolved
R/step-call-pivot_longer.R Show resolved Hide resolved
R/step-call-pivot_longer.R Outdated Show resolved Hide resolved
tests/testthat/_snaps/step-call-pivot_longer.md Outdated Show resolved Hide resolved
@mgirlich
Copy link
Collaborator

mgirlich commented Mar 5, 2021

In hindsight of making tidyr::pivot_longer_spec() and tidyr::build_longer_spec() generics (see tidyverse/tidyr#1101) it would probably make sense to internally split this function into dtplyr_pivot_longer_spec() and dtplyr_build_longer_spec(). This makes it easier to convert them to the corresponding generics as soon as they are converted to S3 methods. The same holds true for pivot_wider()

@hadley
Copy link
Member

hadley commented Mar 7, 2021

When I document this locally, I don't get the \method{} syntax in the .Rd files? Are you doing something special to make this work?

@markfairbanks
Copy link
Collaborator Author

I was actually running into the same thing when I just tried, but it looks like you just have to run library(tidyr). I must have had tidyr loaded in the session to do some tests when I was documenting them originally.

@hadley
Copy link
Member

hadley commented Mar 8, 2021

Ah of course; and that never happens for me because I always run in a clean session.

@hadley
Copy link
Member

hadley commented Mar 8, 2021

Used a hack from @gaborcsardi to always attach tidyr when documenting.

@hadley hadley merged commit daea473 into tidyverse:master Mar 8, 2021
@hadley
Copy link
Member

hadley commented Mar 8, 2021

Thanks!

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.

3 participants