TypedTables.jl icon indicating copy to clipboard operation
TypedTables.jl copied to clipboard

Copying FlexTable column breaks "filter!" function

Open stuartthomas25 opened this issue 3 years ago • 3 comments

Assigning one column of a FlexTable to another new column breaks the filter! function with a cryptic error message. I discovered that the new column shares the same memory as the old column, so that when filter! parses the new column, it is already deleted.

Minimal Working Example:

using TypedTables
data = FlexTable(x=[1,2,3])
data.y = data.x
filter!(r->r.x<3, data)

Error:

ERROR: BoundsError: attempt to access 2-element Vector{Int64} at index [3]
Stacktrace:
 [1] _deleteat!
   @ ./array.jl:959 [inlined]
 [2] deleteat!
   @ ./array.jl:1434 [inlined]
 [3] (::TypedTables.var"#84#85"{UnitRange{Int64}})(col::Vector{Int64})
   @ TypedTables ~/.julia/packages/TypedTables/zfbS2/src/FlexTable.jl:185
 [4] map
   @ ./tuple.jl:222 [inlined]
 [5] map(::Function, ::NamedTuple{(:x, :y), Tuple{Vector{Int64}, Vector{Int64}}})
   @ Base ./namedtuple.jl:208
 [6] deleteat!(t::FlexTable{1}, i::UnitRange{Int64})
   @ TypedTables ~/.julia/packages/TypedTables/zfbS2/src/FlexTable.jl:185
 [7] filter!(f::var"#1#2", a::FlexTable{1})
   @ Base ./array.jl:2536
 [8] top-level scope
   @ REPL[5]:1

stuartthomas25 avatar Aug 18 '22 19:08 stuartthomas25

Hmm interesting. It's generally assumed the columns don't alias each other... so I suspect you'd get the same thing with:

using TypedTables
v = [1,2,3]
data = Table(x=v, y=v)
filter!(r -> r.x < 3, data)

To fix this, maybe we'd need to compare the data_ids of the columns or something. This could get tedious and take a while with lots of columns.

Alternatively, we could document that we don't support mutating columns that alias the same underlying data?

andyferris avatar Aug 31 '22 02:08 andyferris

(obviously, to fix your example you would do data.y = copy(data.x) or else use filter instead of filter!)

andyferris avatar Aug 31 '22 02:08 andyferris

Hi Andy,

Trying your example with "Table" yields the same error. Once I discovered the cause of the error, it was fairly easy to fix it using the copy trick you mention; however it took me a long time to realize this was the root cause. Maybe the solution is simply documentation? Or throwing an error in the filter! function when this is detected to get a better error message?

stuartthomas25 avatar Aug 31 '22 13:08 stuartthomas25