JuliaData / DataTables.jl

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

cleanup views and subdatatable functions, add associated tests #48

Closed cjprybol closed 7 years ago

cjprybol commented 7 years ago

The subdatatable/views code was a little wonky. Sometimes view called subdatatable, sometimes subdatatable called back up to view, and other times view would call view again before later calling subdatatable. The code was also relying on a custom Index that was used nowhere else, and seemed unneccessary. Fixes issue that users could only specify single columns to subset on (rather than arrays of columns), and adds tests for datatables and subdatatables to assert view works as expected. See https://github.com/JuliaData/DataTables.jl/pull/46#issuecomment-291047364

nalimilan commented 7 years ago

Looks good to me apart from the comment above. Also thanks for the thorough testing. I guess this includes the fix from https://github.com/JuliaStats/DataFrames.jl/pull/1177?

cjprybol commented 7 years ago

Yes to including the fix from https://github.com/JuliaStats/DataFrames.jl/pull/1177!

cjprybol commented 7 years ago

Coverage drops unrelated here too

nalimilan commented 7 years ago

Thanks!