Conversation
| # Cross join for empty `join_by()` calls | ||
| cross <- n == 0L | ||
|
|
||
| new_join_by( | ||
| exprs = exprs, | ||
| condition = condition, | ||
| filter = filter, | ||
| cross = cross, |
There was a problem hiding this comment.
Felt "right" to be able to remove cross join handling from join_by()
| new_join_by <- function(exprs, condition, filter, cross, x, y) { | ||
| new_join_by <- function(exprs = list(), | ||
| condition = character(), | ||
| filter = character(), | ||
| x = character(), | ||
| y = character()) { |
There was a problem hiding this comment.
Users can never create empty join_by() calls, but we can internally make an empty new_join_by() object, which is useful for cross_join() and our backwards compatible support for by = character()
| if (!is_character(x_names)) { | ||
| abort("`by$x` must evaluate to a character vector.") | ||
| } | ||
| if (!is_character(y_names)) { | ||
| abort("`by$y` must evaluate to a character vector.") | ||
| } |
There was a problem hiding this comment.
Added some more error handling for some obscure support for by = list(x = , y = ), which I don't think many people use but it was causing a weird error without this
| x_out <- vec_rep_each(x_out, times = y_size) | ||
| y_out <- vec_rep(y_out, times = x_size) |
There was a problem hiding this comment.
This should be faster than the previous approach too. No need to actually "locate" the matches using vec_locate_matches().
| y_unmatched <- unmatched$y | ||
|
|
||
| if (cross) { | ||
| # TODO: Remove this section when `by = character()` is defunct |
There was a problem hiding this comment.
Unfortunately we do still need this section in join_rows() for backwards compatible support, we can't just call out to cross_join() or something.
Technically the unmatched = "error" and multiple = "error" args are supposed to do something even when you set by = character(). These aren't meaningful things to do IMO, which is why cross_join() doesn't have these args, but the simplest way to keep backwards compatible support here seemed to be to just continue to let it all run through vec_locate_matches(). I've added tests of the unmatched and multiple interaction with by = character().
I did go ahead and aggregate any usage of cross below into a single if (cross) branch that will be easy to remove in the future.
| is_cross_by <- function(x) { | ||
| identical(x, character()) || identical(x, list(x = character(), y = character())) | ||
| } |
There was a problem hiding this comment.
The 2nd option here is a technically valid way to do a cross join in CRAN dplyr
DavisVaughan
force-pushed
the
feature/cross-join
branch
from
bc2a083 to
2b90517
Compare
2 participants
Closes #6604
This PR implements
cross_join()as an alternative to theby = character()join syntax. It also soft deprecatesby = character(). Further rationale forcross_join()is provided in #6604 (comment).