DataFrameRow: fix iterate, ndims, iterate #1637
Conversation
| ref = ["a", "b", "c"] | ||
| df = DataFrame(permutedims(ref)) | ||
| dfr = df[1, :] | ||
| @test collect(dfr) == ref |
There was a problem hiding this comment.
Also test the type? When you say the eltype is Any, I guess this applies only to heterogeneous column types, right? It would be useful to cover that case too.
(BTW, Vector doesn't promote AFAICT, as it uses a comprehension.)
There was a problem hiding this comment.
I will fix it and test it. Actually it should be narrowest type (as in comprehensions).
What is problematic (and confused me) that in Matrix(df) we use T = reduce(promote_type, eltypes(df)) and probably we should use T = reduce(typejoin, eltypes(df)). Should I fix it?
There was a problem hiding this comment.
I have added eltype for DataFrameRow and now it will be as you wanted (and it is OK to add it given the interface specification).
There was a problem hiding this comment.
See discussion at #1641. Maybe we should use promotion here too.
There was a problem hiding this comment.
As discussed 😄 in #1641, if someone wants a promotion to T one should use Vector{T}(dfr) in my opinion.
There was a problem hiding this comment.
So should we use promote_type here too for consistency with #1641?
There was a problem hiding this comment.
I thought that this was exactly the point we wanted to have a difference here.
Essentially this is the same as:
julia> collect((1, 1.0, true))
3-element Array{Real,1}:
1
1.0
true
julia> [v for v in (1, 1.0, true)]
3-element Array{Real,1}:
1
1.0
true
But I am not too attached to what we have now. But it we want to do the promotion here we should define eltype (and this is exactly what I have removed some 2 commits ago 😄).
There was a problem hiding this comment.
Maybe we should keep collect as-is for consistency with tuples, but have convert use promotion to be consistent with data frames?
There was a problem hiding this comment.
This is what we do. But it is implemented in #1640.
There was a problem hiding this comment.
Ah, right, too many PRs... :-p
|
I also needed to add one more |
Co-Authored-By: bkamins <bkamins@sgh.waw.pl>
Clean-up of
DataFrameRowAPI.Notice that
collect(dfr)will haveeltypeset toAny(this is the standard mechanics), whileVector(dfr)doespromote_type(and possibly makes a conversion - this is a behavior inherited from eralier implementation ofconverttoArrayforDataFrame).