ARROW-13995: [R] Bindings for join node#11155
Closed
nealrichardson wants to merge 7 commits intoapache:masterfrom
Closed
ARROW-13995: [R] Bindings for join node#11155nealrichardson wants to merge 7 commits intoapache:masterfrom
nealrichardson wants to merge 7 commits intoapache:masterfrom
Conversation
nealrichardson
force-pushed
the
join-dplyr
branch
from
df4c33f to
e947054
Compare
nealrichardson
changed the title
nealrichardson
force-pushed
the
join-dplyr
branch
from
8cf93e6 to
ec85e9f
Compare
jonkeane
force-pushed
the
join-dplyr
branch
from
ec85e9f to
174450c
Compare
jonkeane
reviewed
Sep 28, 2021
r/R/dplyr-join.R
Outdated
Member
There was a problem hiding this comment.
This is not a comment on this code, but it never occurred to me that it's a bit funny that by isn't tidy-evaled and must be character strings (in dplyr!)
Member
There was a problem hiding this comment.
It makes sense for the by = c(col_x = "col_y") interface, but a bit(?) odd for by = "col" case
r/tests/testthat/test-dplyr-join.R
Outdated
Member
There was a problem hiding this comment.
I can do this when I rebase, but should we have tests for the error handling as well (e.g. columns named but not in x or y, etc.)?
Member
Author
There was a problem hiding this comment.
Oh yeah, there are lots of missing tests, and these datasets aren't ideal for testing joins. We may want to just use the datasets that dplyr uses in its tests or something.
Co-authored-by: Jonathan Keane <[email protected]>
jonkeane
force-pushed
the
join-dplyr
branch
from
2ea4435 to
c922dff
Compare
ViniciusSouzaRoque
pushed a commit
to s1mbi0se/arrow
that referenced
this pull request
Oct 20, 2021
This is based on apache#11150. Among the issues observed: * Dictionary columns aren't allowed even in the left data, though you can first `Project` to remove them * Duplicate column names aren't allowed at all, even though there is a provision for deduping with a prefix Closes apache#11155 from nealrichardson/join-dplyr Lead-authored-by: Neal Richardson <[email protected]> Co-authored-by: Jonathan Keane <[email protected]> Signed-off-by: Jonathan Keane <[email protected]>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
This is based on #11150.
Among the issues observed:
Projectto remove them