Skip to content
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

no.dups argument to merge.data.table #2653

Merged
merged 4 commits into from
Mar 5, 2018
Merged

no.dups argument to merge.data.table #2653

merged 4 commits into from
Mar 5, 2018

Conversation

sritchie73
Copy link
Contributor

Follow-up to the accepted PR #2631

On Saturday Feb 24th 2018 (NEWS) the corresponding patch for PR #2631 to base R merge.data.frame() was merged into R-devel.

In this update they have added a no.dups argument to merge.data.frame(). When TRUE (default), this will add the corresponding suffixes to column names iny that share a name with by.x.

This pull request adds this argument and functionality to merge.data.table() to make it consistent with merge.data.frame(), replacing the behaviour of the accepted PR #2631 which made merge.data.table() add suffixes to both the by.x and y columns that shared a name.

merge.data.frame has now added an additional argument, no.dups,
to handle cases where by.x != by.y && any(by.x %in% names(y)).

The current version of merge.data.table was updated so that the
'suffixes' were added to both the by.x and y column in the joined
data.table. However, base R have decided to only add the suffix
to the column in Y for backwards compatibility.
@codecov-io
Copy link

codecov-io commented Mar 3, 2018

Codecov Report

Merging #2653 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2653      +/-   ##
==========================================
+ Coverage   93.19%   93.19%   +<.01%     
==========================================
  Files          61       61              
  Lines       12204    12205       +1     
==========================================
+ Hits        11373    11374       +1     
  Misses        831      831
Impacted Files Coverage Δ
R/merge.R 94.8% <100%> (+0.06%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cb0b500...02f46cf. Read the comment docs.

Copy link
Member

@mattdowle mattdowle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was quick! Great.

@mattdowle mattdowle merged commit bd0d37a into Rdatatable:master Mar 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants