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

Fix for dev dplyr #346

Merged
merged 7 commits into from
Feb 8, 2023
Merged

Conversation

DavisVaughan
Copy link
Contributor

This PR makes your package compatible with the next version of dplyr:

  • if_else() now propagates names so we need to unname where they aren't expected
  • na_if() now coerces y to the type of x so we now need to convert x to character early in a place
  • You had a place where str_detect() could return >1 values inside an if() so I wrapped that in any()
  • case_when() and if_else() now retain the type of matrices rather than coercing them to vectors, so they need to be coerced to vector early
  • distinct() now retains the ordering supplied by the user, so I've reordered with a select() early on to ensure this is compatible with both dev and CRAN dplyr
  • There are a few places where I switched to expect_no_message() or expect_snapshot() due to new warnings that appear in the join functions. The join functions in dplyr (like left_join()) now return a warning by default when a row in x matches multiple rows in y. While this is typical SQL behavior, it is often unexpected during data analysis (many people don't even know it is possible), so we've decided to make this a warning. In dplyr 1.1.0, you silence this warning with multiple = "all".

We plan to submit dplyr 1.1.0 on January 27th.

This should be compatible with both dev and CRAN dplyr. It would help us out if you could go ahead and send a patch version of your package to CRAN ahead of time! Thanks!

Copy link
Contributor

@statasaurus statasaurus left a comment

Choose a reason for hiding this comment

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

Thank you so much

@statasaurus statasaurus merged commit f72ab81 into GSK-Biostatistics:main Feb 8, 2023
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.

None yet

2 participants