Skip to content

Commit

Permalink
Fix sort with unused levels (#273)
Browse files Browse the repository at this point in the history
The code did not handle correctly the case when unused levels were present at the front.
This seems to have become easier to trigger since the index vs. levels distinction
was dropped.
  • Loading branch information
nalimilan authored May 19, 2020
1 parent bbd3cc0 commit 3fb5245
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 7 deletions.
13 changes: 7 additions & 6 deletions src/array.jl
Original file line number Diff line number Diff line change
Expand Up @@ -943,16 +943,17 @@ function Base.sort!(v::CategoricalVector;

# compute the order in which to read from counts
ord = Base.Sort.ord(lt, by, rev, order)
index = eltype(v) >: Missing ? [missing; v.pool.valindex] : v.pool.valindex
seen = counts .> 0
anymissing = eltype(v) >: Missing && seen[1]
perm = sortperm(view(index, seen), order=ord)
nzcounts = counts[seen]
levs = eltype(v) >: Missing ? [missing; v.pool.valindex] : v.pool.valindex
sortedlevs = sort!(Vector(view(levs, seen)), order=ord)
levelsmap = something.(indexin(sortedlevs, levs))
j = 0
refs = v.refs
@inbounds for ref in perm
tmpj = j + nzcounts[ref]
refs[(j+1):tmpj] .= ref - anymissing
@inbounds for i in eachindex(levelsmap)
ref = levelsmap[i]
tmpj = j + counts[ref]
refs[(j+1):tmpj] .= ref - (eltype(v) >: Missing)
j = tmpj
end

Expand Down
6 changes: 5 additions & 1 deletion test/13_arraycommon.jl
Original file line number Diff line number Diff line change
Expand Up @@ -926,7 +926,11 @@ end
cv = categorical(v)
levels!(cv, ["b", "a", "c", "d"])
@test sort(cv, rev=rev)
["b", "a", "c", "d", missing][sort([5, 1, 2, 3, 4][cv.refs .+ 1], rev=rev)]
["b", "a", "c", "d", missing][sort([5; 1:4][cv.refs .+ 1], rev=rev)]

levels!(cv, ["x", "z", "b", "a", "y", "c", "d", "0"])
@test sort(cv, rev=rev)
["x", "z", "b", "a", "y", "c", "d", "0", missing][sort([9; 1:8][cv.refs .+ 1], rev=rev)]

cv = categorical(v)
@test sort(cv, rev=rev, lt=(x, y) -> isless(y, x))
Expand Down

0 comments on commit 3fb5245

Please sign in to comment.