Skip to content
This repository was archived by the owner on May 4, 2019. It is now read-only.

test/sort.jl broken on current Julia master #208

Closed
martinholters opened this issue Jul 21, 2016 · 6 comments
Closed

test/sort.jl broken on current Julia master #208

martinholters opened this issue Jul 21, 2016 · 6 comments

Comments

@martinholters
Copy link
Contributor

Since JuliaLang/julia@a4e3cd2, typed_vcat (called from vcat) explicitly converts its arguments to Arrays. This breaks isequal(sort(da), [DataArray(sort(dropna(da))); DataArray(T, nna)]) since the [...; ...] tries to put NAtypes into an Array{Float64,1}.

@tkelman
Copy link
Contributor

tkelman commented Jul 22, 2016

If the best solution to this is to merge revert JuliaLang/julia#17079 for 0.5, please let us know quickly.

@ararslan
Copy link
Member

Someone else surely would know better than I would (maybe @andreasnoack? he knows everything) but I'm not sure what we could do in the short term on our end to accomodate that change.

@andreasnoack
Copy link
Member

The reasoning behind JuliaLang/julia#17079 was that full(a) would always be convert(Array, a) but that is clearly not true so the solution should either be to revert it modify it but given how close we are to 0.5, reverting might be the right choice.

@martinholters
Copy link
Contributor Author

martinholters commented Jul 25, 2016

Actually, the fact that, for some da::DataArray{T}, isa(da, AbstractArray{T}) and hence eltype(da) == T, but potentially !isa(da[idx], T) looks like it is breaking a lot of assumptions. Funny it doesn't trigger more errors.

That said, the simplest solution for this particular issue might be adding specialized methods to vcat (and probably the other cat methods, too).

Or, even simpler, disallow cat'ing DataArrays and rewrite the offending tests 😄

EDIT: I had completely missed JuliaLang/julia#17564...

@nalimilan
Copy link
Member

This is annoying. I think this shows that DataArrays are fundamentally broken as they don't respect the element type they declare. OTOH breaking DataArrays in Julia 0.5 would be a serious issue since the replacement isn't completely ready yet. So either we're able to override all cat methods for DataArrays, or we should keep JuliaLang/julia#17079 reverted until 0.6.

@andreasnoack
Copy link
Member

JuliaLang/julia@a4e3cd2 Has been reverted so this is not an issue anymore. I'm wondering if the element type of a DataArray should be Union{T,NAType}. That seems right to me and would work with JuliaLang/julia@a4e3cd2

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

No branches or pull requests

5 participants