-
-
Notifications
You must be signed in to change notification settings - Fork 17
data_to_wide(): disable arg values_fill, do not drop empty columns, allow select helpers
#646
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
|
All sound good to me, I hadn't noticed the different behavior from tidyr on filling missing values. Instead of removing |
|
Haha, our internal select-helper only works when the variable is named |
|
Ah, found the solution, via |
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.
Pull Request Overview
This PR revises the data_to_wide() function to remove the non-functional values_fill argument, preserve empty columns after widening, and add support for select-helpers in values_from.
- Remove defunct
values_fillfunctionality and deprecate the argument - Stop removing empty columns created during widening process to align with
tidyr::pivot_wider()behavior - Add select-helper support for
values_fromparameter with newignore_caseandregexarguments
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| R/data_to_wide.R | Core function updates: remove values_fill logic, add validation, implement select-helpers |
| man/data_to_wide.Rd | Update documentation to reflect argument changes and new functionality |
| tests/testthat/test-data_to_wide.R | Replace values_fill tests with deprecation tests, add validation and empty column preservation tests |
| NEWS.md | Document breaking changes and new features |
| DESCRIPTION | Version bump |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
Most changed lines are actually Air-styler-stuff. |
Can you revert those? It makes the PR hard to review. I'd like to use Air everywhere in easystats, let's just discuss that in the easystats repo and then make a PR dedicated to that if we choose to use it. |
|
Ok, done. Ready for review |
etiennebacher
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.
Thank you, LGTM overall, just a few small comments to address.
data_to_wide()data_to_wide(): disable arg values_fill, do not drop empty columns, allow select helpers
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #646 +/- ##
==========================================
+ Coverage 91.73% 91.83% +0.09%
==========================================
Files 76 76
Lines 6410 6476 +66
==========================================
+ Hits 5880 5947 +67
+ Misses 530 529 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
values_fillargument, as this never worked as probably intendedvalues_fromThis one replaces #645