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

Closes #172 Updates to align with updates to tidyselect and dplyr functionality #174

Merged
merged 2 commits into from
Jun 15, 2023

Conversation

kodesiba
Copy link
Collaborator

@kodesiba kodesiba commented May 9, 2023

Release Description

Hotfix release

Milestone

Milestone: N/A
Closes #172

Release Checklist

  • Version number has been updated
  • Description file updated with New Developers (if applicable)
  • NEWS.md has been updated
  • README.md has been updated (if applicable)
  • Vignettes have been updated (if applicable)
  • Ensure all unit tests are passing
  • GitHub actions on this PR are all passing
  • Run spelling::spell_check_package() and address any issues
  • Draft GitHub release created using automatic template and updated with additional details. Remember to click "release" after PR is merged.

@kodesiba kodesiba self-assigned this May 9, 2023
@kodesiba kodesiba marked this pull request as draft May 9, 2023 16:45
@bms63 bms63 marked this pull request as ready for review May 9, 2023 17:51
@parmsam-pfizer
Copy link
Collaborator

Do we need distinct(across()) or distinct(across(.cols = everything(), fns = NULL))? It looks like distinct() would suffice since it gets distinctness using all variables in the dataframe. Alternatively, if we need to include the tidyselect part then this works: distinct(across(.cols = everything()))
https://github.com/pharmaverse/logrx/pull/174/files#diff-be75f6398b6213cfe50e1318d8488ba76de0c0cefddbaae032844bad84fe8492R200-R201

@kodesiba
Copy link
Collaborator Author

@nicholas-masel any thoughts here? Not fully sure how everything is working in this function and I think you were the one who wrote this part.

NEWS.md Outdated
@@ -1,3 +1,7 @@
# logrx 0.2.2

- hotfix for update to `across()` and `.data$var` updates in source packages (#172)
Copy link
Collaborator

Choose a reason for hiding this comment

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

mention requirement for dplyr

Copy link
Collaborator

@nicholas-masel nicholas-masel left a comment

Choose a reason for hiding this comment

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

@parmsam-pfizer is correct. This can just use distinct() without across().

NAMESPACE Outdated Show resolved Hide resolved
R/get.R Outdated Show resolved Hide resolved
R/get.R Outdated Show resolved Hide resolved
@bms63 bms63 changed the title Updates to align with updates to tidyselect and dplyr functionality Closes #172 Updates to align with updates to tidyselect and dplyr functionality Jun 15, 2023
@bms63 bms63 merged commit 65fdb24 into main Jun 15, 2023
@bms63 bms63 deleted the hotifx-172 branch June 15, 2023 16:08
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.

Bug: Warning message "Using across() without supplying .cols was deprecated in dplyr 1.1.0."
4 participants