-
Notifications
You must be signed in to change notification settings - Fork 370
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
Update join #536
Update join #536
Conversation
end | ||
|
||
function addx!(d::DataFrame, x::DataFrame, times::Int, each::Int) | ||
for c in x |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we explicitly iterate over columns? This kind of iteration is something I'd like to remove from DataFrames.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Definitely, but I'm still fuzzy on what functions should and shouldn't access. Like this?
for n in names(x)
d[n] = rep(x[n], times, each)
end
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was going to suggest using eachcol
, but I just realized that it doesn't give the column names.
What about giving eachcol
the same behavior as the default iterator (i.e., returning a tuple of (colname, coldata)
), and deprecating the default iterator?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like Kevin's suggestion.
I implemented @kmsquire's suggestion for @johnmyleswhite Natural joins are now being deprecated. And |
I switched join from being a wrapper back to the original form to make it ready to merge. Seems good to me. |
Let's merge this. One way we could handle the dispatch issue that prompted your wrapping is to make |
Thanks for doing this, Sean! And thanks for slugging through all my comments. |
Of course! |
Nice addition indeed! |
Deprecate natural joins (user must specify join key).
Add
kind = :cross
.This is a product of discussion in #531.