-
Notifications
You must be signed in to change notification settings - Fork 367
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
minor fixes #916
minor fixes #916
Conversation
Useful for working with DataFrameRow objects, allowing, for example, a natural selection-partition-accumulation idiom when iterating over an eachrow(dataframe).
@@ -5,7 +5,7 @@ immutable DataFrameRow{T <: AbstractDataFrame} | |||
end | |||
|
|||
function Base.getindex(r::DataFrameRow, idx::AbstractArray) | |||
return DataFrameRow(r.df[[idx]], r.row) | |||
return DataFrameRow(r.df[collect(idx)], r.row) |
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.
Can't you simply remove the innner [ ]
here? getindex(::DataFrame, idx)
should take care of the rest.
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 just wanted to make a minimal modification originally, but I see your point. I removed collect
and updated the PR. Tests run fine, so if it had a role it is not immediately apparent.
Suggestion by @nalimilan.
See #912 about fixing the tests. Should be good to merge despite the failures. |
Sorry for the delay. Would you rebase now that the tests pass? Then should be good to merge. |
Certainly, will do it within the next few days. |
still relevant? |
Bump. |
Thank you for the PR proposal. The
Therefore from this PR we are left with |
@tpapp Given v0.19.0 changes I would close it. The only thing that you propose that is not covered is CC @nalimilan |
[]
being used for concatenationsimilar
methods forDataFrameRow
ssetindex!
forDataFrameRows
s as values, settingDataFrame
sI found the latter two idiomatic when used in loops with
eachrow
. Tests are of course included.