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.

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

Copying FlexTable column breaks "filter!" function #97

Open
stuartthomas25 opened this issue Aug 18, 2022 · 3 comments
Open

Copying FlexTable column breaks "filter!" function #97

stuartthomas25 opened this issue Aug 18, 2022 · 3 comments

Comments

@stuartthomas25
Copy link

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
@andyferris
Copy link
Member

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
Copy link
Member

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

@stuartthomas25
Copy link
Author

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants