Some methods from the standard library don't fulfil their contract

[tomasaschan]

unique(A::DataArray) reorders items

According to the spec, unique(A) should not re-order elements. However, it does for DataArrays:

julia> A = sort(rand(100)); # Get some data, sorted for simplicity
julia> DA = DataArray(A);
julia> all(diff(unique(A)) .>= 0) # Test that this works for regular arrays
true
julia> all(diff(unique(DA)) .>= 0) # It doesn't work for DataArrays!
false
julia> all(diff(unique(DA.data)) .>= 0) # It DOES work for the underlying data
true

Note that this is a problem even without any 'NA` values.

For sorted data this isn't really a big problem, as I can just re-sort it again, but for data that is not sorted but where ordering is important (such as experimental observations) that's not an option either.

sort(A::DataArray) throws error when missing values

Using A and DA as above, I get

julia> sort(DA)
100-element DataArray{Float64,1}:
 0.000314079
 0.00973866 
 # etc...

i.e. it Just Works(TM). However, if there are missing values:

julia> DA[35] = NA;
julia> sort(DA)
ERROR: no method isnan(ForwardOrdering, NAtype)
 in nans2right! at sort.jl:409
 in fpsort! at sort.jl:428
 in sort! at sort.jl:441
 in sort at sort.jl:334

I'd prefer if it "just worked" and placed all NA values last...

[johnmyleswhite]

Why does unique preserve order in Base?

I think we can add a flag to let sort work, but it seems really strange that sort would produce an output in the face of NA, but median would not.

[nalimilan]

I guess returning elements in order of appearance usually doesn't cost much to the implementation of unique, and in some occasions it can be useful (think of observations that are already ordered by some variable in the data set). Base's unique is very simple: https://github.com/JuliaLang/julia/blob/867072608ad60385e34b8b735eadb7dbedfc7ee1/base/set.jl#L94

From a some quick profiling, it doesn't seem that the DataArray version (which uses a Dict) is faster, rather the contrary (of course, it does a little more work too).

[johnmyleswhite]

Ok. Let's crib the Base implementation then.

[nalimilan]

I can give it a try if you like.

[tomasaschan]

Re sort and e.g. median: when calculating the median of a set of values, all values are important, and the median will change depending on what the missing value should be, should it be filled in again. When sorting, on the other hand, we only care about two values at any time, and ignore the rest - the interesting question is just "is a smaller or larger than b?" - so the result of sorting all the non-missing values isn't dependent on the missing ones.

I can imagine (at least) two behaviors of sort that would make sense to me:

• Sort all values, but never move an NA value - i.e. sort(DataArray([2 1 NA 4 NA 5 3])) would return [1 2 NA 3 NA 4 5]
• Place all NA values last, i.e. return [1 2 3 4 5 NA NA]

The latter is probably easier to implement, and easier to make fast. I don't know if there is a use case for the former, but at least that behavior wouldn't surprise me.

[nalimilan]

I think when sorting NAs should appear last, just like NaNs.

Regarding unique, I have code for DataArrays, but I'm wondering what should be done for PooledDataArrays. In their case, it is very fast to get the pool of values as it's already stored, but the current implementation needs to go over the vector to check whether NAs are present, which is kind of silly (this could easily be fixed by storing NA in the pool in that case, or by having a special boolean). Furthermore, in order to place NA in its order of appearance in the data, rather than at the end of the levels, as is done currently, unique would need to do more work (i.e. go over the vector until all levels have been encountered to find out the order), which we're not going to be able to optimize away in the future.

So I can see three choices:

1. Consider that unique is not efficient for PDAs, but that we don't care as levels is the most common function.
2. Consider we care, and:
   a) Put NAs at the end of levels for both DAs and PDAs.
   b) Put NAs at the end for PDAs (because that's where it makes a difference), but in the order of appearance for other DAs.

I'm inclined towards 2a and 1, in that order (not of appearance, obviously ;-).

[johnmyleswhite]

2a sounds good to me.

[nalimilan]

Here you go. Performance seems to be on par with Base, just slightly slower.

[nalimilan]

Thinking about PDAs again, we're going to have to choose anyway between a slow unique which will respect the order of input values, or a fast unique which would essentially be [levels, NA]. For now, we have the latter, and this still doesn't honor the "contract" from Base documentation. Maybe that's not a problem because PDAs are different when it comes to levels; hard to tell...

[simonster]

Unless we're going to make PooledDataArrays immutable by convention, I don't think there's any guarantee that the pool is already sorted. Consider:

julia> a = @pdata ["a", "b", "c"];

julia> a[1], a[2] = a[2], a[1];

julia> a
3-element PooledDataArray{ASCIIString,Uint32,1}:
 "b"
 "a"
 "c"

julia> a.pool
3-element Array{ASCIIString,1}:
 "a"
 "b"
 "c"

[simonster]

(It is also possible that there are entries in the pool that are not present in the data. I'm not sure whether levels needs to check for this, but unique probably should.)

[johnmyleswhite]

I had long ago proposed ensuring both of those invariants and still think it could be useful.

[nalimilan]

Whether or not we keep the pool in sync with the actual values in the array, the order of levels will still be allowed to differ from that of the data, because that's so useful.

So if we really want unique(::PooledDataArray) to return levels in their order of appearance in the data, we may as well show NAs in that order too, because that doesn't add any cost.