JuliaML / TableTransforms.jl

Transforms and pipelines with tabular data in Julia
https://juliaml.github.io/TableTransforms.jl/stable
MIT License
103 stars 15 forks source link

Fix bug in `Tables.getcolumn(t::TableSelection, i::Int)` #150

Closed eliascarv closed 1 year ago

eliascarv commented 1 year ago

Code:

julia> using TableTransforms

julia> using Tables

julia> using TypedTables

julia> table = Table(
           a = 1:5,
           b = 6:10,
           c = 11:15,
           d = 16:20
       );

julia> transf = Select(:a => :x, :d => :y)
Select transform
├─ colspec = [:a, :d]
└─ newnames = [:x, :y]

julia> newtable, cache = apply(transf, table);

julia> newtable
TableSelection
┌───────┬───────┐
│     x │     y │
│ Int64 │ Int64 │
├───────┼───────┤
│     1 │    16 │
│     2 │    17 │
│     3 │    18 │
│     4 │    19 │
│     5 │    20 │
└───────┴───────┘

master:

julia> Tables.getcolumn(newtable, 1)
ERROR: type NamedTuple has no field x
Stacktrace:
 [1] getproperty
   @ .\Base.jl:38 [inlined]
 [2] getcolumn
   @ C:\Users\Dev01\.julia\packages\Tables\T7rHm\src\Tables.jl:102 [inlined]
 [3] getcolumn(t::TableTransforms.TableSelection{Table{NamedTuple{(:a, :b, :c, :d), NTuple{4, Int64}}, 1, NamedTuple{(:a, :b, :c, :d), NTuple{4, UnitRange{Int64}}}}, NamedTuple{(:a, :b, :c, :d), NTuple{4, UnitRange{Int64}}}}, i::Int64) 
   @ TableTransforms c:\Users\Dev01\Documents\Elias\TableTransforms.jl\src\tableselection.jl:46
 [4] top-level scope
   @ REPL[9]:1

This PR:

julia> Tables.getcolumn(newtable, 1)
1:5

julia> Tables.getcolumn(newtable, 2)
16:20
juliohm commented 1 year ago

Tables.getcolumn is only called with the return of Tables.columns right? Can you please double check?

juliohm commented 1 year ago

We need to make sure to not implement column functions on the table type, but instead on the return of Tables.columns.