JuliaData / DataTables.jl

(DEPRECATED) A rewrite of DataFrames.jl based on Nullable
Other
29 stars 11 forks source link

update vcat to mimic Base.vcat and enhance promotion rules of mixed column type #45

Closed cjprybol closed 7 years ago

cjprybol commented 7 years ago

The current vcat operates on Arrays of DataTables DataTable[], while Base.vcat utilizes slurping to capture any number of inputs as a tuple or argument to concatenate. This PR changes vcat to follow Base.vcat's lead and utilizes a function structure of vcat(dts::AbstractDataTable...). Uses @nalimilan's @generated function to implement a new type of AbstractArray promotion rule that ideally can be tested here and transfered into Base in the near future https://github.com/JuliaLang/julia/pull/20815. This also removes the prior assumptions that were made about how to join datatables that were out of order or had columns that were only present in some of the arguments and not others.

cjprybol commented 7 years ago

Thanks for the review! I'll get to these edits soon. re: slurping/splatting, I'm not sure if I used the right one. Both descriptions are used in the docs, but I'll say something more explicit in the commit to avoid any ambiguity.

nalimilan commented 7 years ago

Funny, I didn't remember that term. It's fine then.

cjprybol commented 7 years ago

That should address all of the comments. I added more tests to assert that the array promotion was working as intended, and a couple tests for vcatting 3 or more datatables to make sure my offset copy!s were working as intended. I think I'm finally starting to get squashing/rebasing/amending commits too. Coverage drops seem unrelated and should be addressed by https://github.com/JuliaData/DataTables.jl/pull/31.

cjprybol commented 7 years ago

@nalimilan

cjprybol commented 7 years ago

No worries! It's not that easy to keep up with the rate at which you do manage to review either. Thanks, as always, for putting in the time to help me improve these PRs.

cjprybol commented 7 years ago

Coverage drop doesn't seem related here either. Several functions flagged by Coveralls are handled by https://github.com/JuliaData/DataTables.jl/pull/31

cjprybol commented 7 years ago

Thanks!