WIP: Fix vcat to handle type promotion and preserve PDAs

[garborg]

Compared to the alternatives outlined in the gist in #732, this is looking 4-80x faster (contingent on a forthcoming edit to copy! in DataArrays).

[tshort]

Thanks, Sean! Stefan used to jokingly suggest submitting half-baked solutions given that might spur Jeff to "implement it right". Now I feel like that guy:)

This should take care of the type promotion. I was also hoping that vcat for columns would take care of promotion of column containers. For example, if you vcat columns that are both PooledDataArrays, the result will be a DataArray. The same is true for Arrays and other AbstractArray types. I don't see any mechanism for that in the DataArray package, however, so that seems like a bigger challenge.

[garborg]

Oh yes, I had thought about that for combining Nullable/DataArrays with other AbstractArrays, but had totally forgotten about PooledDataArrays, which seems more immediate. I'm interested to see if container promotion lands in base soon -- I'm not sure what we should do with corner cases (e.g. promote_type(DataArray, PooledDataArray), promote_type(NullableArray, DataArray)) -- ideas? Maybe for now, just calling similar on the first AbstractDataArray container type if found, otherwise, on the first container type?

[nalimilan]

Maybe for now, just calling similar on the first AbstractDataArray container type if found, otherwise, on the first container type?

I think it would be more reliable to define promotion rules. DataArray and PooledDataArray should give a DataArray, as you can assume that if it made sense to store the DataArray as a PooledDataArray, the user who have converted it (e.g. it contains too many different values).

As for DataArray and NullableArray, shouldn't one replace the other at some point anyway?

[garborg]

Makes sense re: DataArray and PooledDataArray, makes sense -- I was actually thinking of the hypothetical case where what I called PooledDataArray was something holding cardinal or ordinal data.

Re: DataArray & NullableArray, maybe, but generally, there will be more container types that don't exist yet, and I suppose I just can't picture a world where they'll all work seamlessly together. I wanted to say we should throw an error in cases where promote_type falls back to an abstract type, but that would be wrong given two different AbstractArray subtypes for which similar returns a Array.

[johnmyleswhite]

Yes, DataArray needs to be completely removed once NullableArray arrives. There's no way that NA and Nullable{T}() can safely co-exist.

[garborg]

Updated to preserve PDAs. As with the other proposed solutions, but unlike before, now behaves like vcat for Base and DataArrays -- i.e. vcat([1, 2], @data([NA, 4]) fails.

[coveralls]

Coverage increased (+0.06%) when pulling 2293996 on vcat into 23e090d on master.

[nalimilan]

@garborg Why should it fail? Rather, I think that combining Array and DataArray should always give a DataArray, just like promoting a float and a complex gives a complex. The more general type should win, as it's the only one able to hold values from both types.

[garborg]

Agreed that it probably should work, but everywhere. We've always gone with parity with Base methods -- it should not fail on DataFrames if it should not fail on vcat(::Array, ::DataArray). Tom has opened a DataArrays issue for that, though it may be a Base issue. Holding up the the ability to promote eltypes or accept DataFrames that have PooledDataArrays (which work in other vcat implementations) seems like putting the burden in the wrong place.

[garborg]

(P.S. Tests have already been added, commented out, for if/when it works universally, in anticipation of someone pushing that through.)