Enforce empty dots when not used #429
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
and the inherited docs
using `V` instead of `v` was silently ignored and now triggers an error due to the new `check_dots_empty()` inside of `vfold_cv()`
the underlying `mc_cv()` and `group_mc_cv()` already enforce empty dots
Don't use the rlang doc for the generic because we don't require those dots to be empty
for the methods, the dots need to actually be empty
which propagates up to `as.data.frame()` as well as `analysis()` and `assessment()`
This was referenced Apr 21, 2023
- `labels()` methods - `dim()` methods - `as.integer()` method
DavisVaughan
approved these changes
Apr 27, 2023
|
This pull request has been automatically locked. If you believe you have found a related problem, please file a new issue (with a reprex: https://reprex.tidyverse.org) and link to this issue. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
closes #427
Generally, this PR checks for empty dots via
rlang::check_dots_empty()whenever they are documented as "not used".For the
tidy()methods, it was documented that the dots are not used. However, that was only true for some of the methods, see e.g., the tests fortidy.vfold_cv()which passedunique_idthrough the dots. Now all general methods (forrsplit,rset,nested_cv) have an explicitunique_indarg and the dots have to be empty. Forvfold_cv, there is no such arg because each observation only gets assigned to one fold (per repeat) and thusunique_indshould not make a difference for the tidy output.The dots are generally documented via the rlang tag. Because that (also) states that the dots must be empty, the dots for a generic are documented with "Not currently used." only (since, for the generic, they do not need to be empty).
Methods for base-R generics like
print()andas.integer()have unchecked dots documented as "Not currently used".Revdeps:
"1a6d222f-b434-4844-972f-805b5fb6f0b7"This breaks 2 revdeps which are both now fixed in their dev version:
initial_split()butcher#259Do we typically add the revdep files to the PR? rsample already has a
revdep/folder under version control, and I'm wondering if I'm adding this here or leaving them untouched until release time.