Factor out eval_relocate() and use dplyr_col_select() in relocate()
#6342
Conversation
|
@lionel- would you mind looking at this since it would probably end up in tidyselect (r-lib/tidyselect#232)? I think it is ok if it is not 100% equivalent to what you'd implement for |
DavisVaughan
force-pushed
the
feature/eval-relocate
branch
from
04d2b92 to
0224ecb
Compare
| has_after <- !quo_is_null(.after) | ||
| n <- length(data) | ||
|
|
||
| before <- as_quosure(before, env = env) |
There was a problem hiding this comment.
I'm a bit on the fence about whether it'd be better to use enquo() for these arguments. But it seems better to be consistent and only take strict arguments in an eval_ function, so I think this is a good solution.
There was a problem hiding this comment.
Yea I was trying to be consistent with expr here, since eval_select() captures expr in the same way with this as_quosure() idea.
I started with enquo() but it felt really strange. This felt better to me
R/relocate.R
Outdated
| } | ||
|
|
||
| if (has_before) { | ||
| # TODO: Use `allow_rename = FALSE`. https://github.com/r-lib/tidyselect/issues/225 |
There was a problem hiding this comment.
This is fixed in the dev version which I plan to release right before the dplyr release.
There was a problem hiding this comment.
But this bug still exists :/ r-lib/tidyselect#221
There was a problem hiding this comment.
I've added this to my list of issues to fix for the dplyr release.
DavisVaughan
force-pushed
the
feature/eval-relocate
branch
from
5862246 to
b31a925
Compare
Closes #5829 (but we'd still need to move this to tidyselect)
Closes #6167
Closes #6341
This PR pulls
eval_relocate()out ofrelocate.data.frame(). The eventual goal would be to move this to tidyselect for other backends to use it (#5829) but this is a good first step.The
beforeandafterarguments ofeval_relocate()take defused input, orNULL, which feels right given thatexprworks basically the same way.eval_relocate()returns a named integer vector the same size asncol(data)(important invariant) that describes how to rearrange the columns ofdata. The names are normally the same as the original data, but renaming can happen inexprso it is possible for them to be different.I fixed a few edge case bugs in the implementation of
eval_relocate(). Mainly around 0-col data frames and what happens when you supply.beforeor.afterbut they result in an empty selection.I've also started using
dplyr_col_select()inrelocate.data.frame()rather than just.data[pos]. This solves #6341 and probably a few other consistency bugs sincedplyr_col_select()has a few extra sanity checks in it.