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.

Sign up for GitHub

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

Jump to bottom

Add Tables.jl interface for DataFrame(Rows|Columns) #2055

Merged
merged 7 commits into from
Dec 16, 2019
Merged

Add Tables.jl interface for DataFrame(Rows|Columns) #2055

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
6ed00bd
Add Tables.jl interface for DataFrame(Rows|Columns)
tkf Dec 15, 2019
9e6b078
Use eval instead of Union
tkf Dec 15, 2019
b294935
Use parent instead of getfield
tkf Dec 15, 2019
5325ce0
Fix a test for DataFrame!(eachrow(df))
tkf Dec 16, 2019
22e32db
Use Union (without alias) instead of eval
tkf Dec 16, 2019
fbeaff4
Materialize as DataFrame(Rows|Columns)
tkf Dec 16, 2019
62ab4b6
Inherit `names` argument in `materializer(::DataFrameColumns)`
tkf Dec 16, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions src/other/tables.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,18 @@ DataFrame!(x::Vector{<:NamedTuple}) =
"`$(typeof(x))` without allocating new columns: use " *
"`DataFrame(x)` instead"))

for T in [DataFrameRows, DataFrameColumns]
@eval begin
Copy link
Member

Choose a reason for hiding this comment

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

Do you really need a loop? Isn't ::Union{DataFrameRows, DataFrameColumns} enough?

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I hadn't seen @bkamins's comment above. I'd just repeat the Union without defining a custom type alias.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm OK with both approaches. I wrote this as @bkamins preferred this approach. Ref: #2055 (comment)

Copy link
Member

Choose a reason for hiding this comment

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

I am OK with both - the @eval approach is used in Base often. But using Union without defining the alias is also OK (I just prefer not to introduce the alias here).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does 22e32db look good?

Tables.istable(::Type{<:$T}) = true
Tables.columnaccess(::Type{<:$T}) = true
Tables.rowaccess(::Type{<:$T}) = true
Tables.columns(itr::$T) = Tables.columns(parent(itr))
Tables.rows(itr::$T) = Tables.rows(parent(itr))
Tables.schema(itr::$T) = Tables.schema(parent(itr))
Tables.materializer(itr::$T) = Tables.materializer(parent(itr))
end
end

IteratorInterfaceExtensions.getiterator(df::AbstractDataFrame) = Tables.datavaluerows(df)
IteratorInterfaceExtensions.isiterable(x::AbstractDataFrame) = true
TableTraits.isiterabletable(x::AbstractDataFrame) = true
18 changes: 9 additions & 9 deletions test/tables.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,15 +56,15 @@ Base.propertynames(d::DuplicateNamesColumnTable) = (:a, :a, :b)
@testset "Tables" begin
df = DataFrame(a=Int64[1, 2, 3], b=[:a, :b, :c])

@testset "basics" begin
@test Tables.istable(df)
@test Tables.rowaccess(df)
@test Tables.columnaccess(df)
@test Tables.schema(df) === Tables.Schema((:a, :b), Tuple{Int64, Symbol})
@test Tables.schema(df) == Tables.schema(Tables.rows(df)) == Tables.schema(Tables.columns(df))
@test @inferred(Tables.materializer(df)(Tables.columns(df))) isa typeof(df)

row = first(Tables.rows(df))
@testset "basics $(nameof(typeof(table)))" for table in [df, eachrow(df), eachcol(df)]
@test Tables.istable(table)
@test Tables.rowaccess(table)
@test Tables.columnaccess(table)
@test Tables.schema(table) === Tables.Schema((:a, :b), Tuple{Int64, Symbol})
@test Tables.schema(table) == Tables.schema(Tables.rows(table)) == Tables.schema(Tables.columns(table))
@test @inferred(Tables.materializer(table)(Tables.columns(table))) isa typeof(df)

row = first(Tables.rows(table))
@test propertynames(row) == (:a, :b)
@test getproperty(row, :a) == 1
@test getproperty(row, :b) == :a
Expand Down