Merge e3f6dda into b0139c1

src/array.jl

@@ -364,15 +364,24 @@ function copyto!(dest::CatArrOrSub{T, N}, dstart::Integer,
     # try converting src to dest type to avoid partial copy corruption of dest
     # in the event that the src cannot be copied into dest
     slevs = convert(Vector{T}, levels(src))
+    dlevs = levels(dest)
     if eltype(src) >: Missing && !(eltype(dest) >: Missing) && !all(x -> x > 0, srefs)
         throw(MissingException("cannot copy array with missing values to an array with element type $T"))
     end
 
-    newlevels, ordered = mergelevels(isordered(dest), levels(dest), slevs)
-    # Orderedness cannot be preserved if the source was unordered and new levels
-    # need to be added: new comparisons would only be based on the source's order
-    # (this is consistent with what happens when adding a new level via setindex!)
-    ordered &= isordered(src) | (length(newlevels) == length(levels(dest)))
+    newlevels, ordered = mergelevels(isordered(dest), dlevs, slevs)
+    if isordered(dest) && (length(newlevels) != length(dlevs))
+        # Uncomment this when removing deprecation
+        # throw(OrderedLevelsException(newlevels[findfirst(!in(Set(dlevs)), newlevels)],
+        #                              dlevs))
+        Base.depwarn("adding new levels to ordered CategoricalArray destination " *
+                     "will throw an error in the future", :copyto!)
+        ordered &= isordered(src) | (length(newlevels) == length(dlevs))
+    end
+    # Exception: empty pool marked as ordered if new value is ordered
+    if isempty(dlevs) && isordered(src)
+        ordered = true
+    end
     if ordered != isordered(dest)
         isa(dest, SubArray) && throw(ArgumentError("cannot set ordered=$ordered on dest SubArray as it would affect the parent. Found when trying to set levels to $newlevels."))
         ordered!(dest, ordered)
@@ -409,11 +418,12 @@ copyto!(dest::CatArrOrSub, src::CatArrOrSub) =
 copyto!(dest::CatArrOrSub, dstart::Integer, src::CatArrOrSub) =
     copyto!(dest, dstart, src, 1, length(src))
 
-@static if VERSION >= v"0.7.0-DEV.3208"
-    using Future
-    Future.copy!(dest::CatArrOrSub, src::CatArrOrSub) =
-        copyto!(dest, 1, src, 1, length(src))
+if VERSION >= v"1.1"
+    import Base: copy!
+else
+    import Future: copy!
 end
+copy!(dest::CatArrOrSub, src::CatArrOrSub) = copyto!(dest, 1, src, 1, length(src))
 
 similar(A::CategoricalArray{S, M, R}, ::Type{T},
         dims::NTuple{N, Int}) where {T, N, S, M, R} =

src/pool.jl

@@ -199,6 +199,10 @@ end
             throw(OrderedLevelsException(level, pool.levels))
         end
         newlevs, ordered = mergelevels(isordered(pool), pool.levels, level.pool.levels)
+        # Exception: empty pool marked as ordered if new value is ordered
+        if length(pool) == 0 && isordered(level.pool)
+            ordered!(pool, true)
+        end
         levels!(pool, newlevs)
     end
     get!(pool, get(level))

test/07_levels.jl

@@ -283,6 +283,17 @@ using CategoricalArrays: DefaultRefType, levels!
     p2 = CategoricalPool(Any['a', 'b', 'x'])
     @test get!(p1, p2[1]) === UInt32(1)
     @test get!(p1, p2[3]) === UInt32(4)
+
+    # get! with ordered CategoricalValue marks unordered empty pool as ordered
+    p1 = CategoricalPool(['b', 'c', 'a'])
+    ordered!(p1, true)
+    p2 = CategoricalPool(Char[])
+    @test get!(p2, p1[1]) === UInt32(1)
+    @test isordered(p2)
+    # But push! does not
+    p2 = CategoricalPool(Char[])
+    @test push!(p2, p1[1]) === p2
+    @test !isordered(p2)
 end
 
 @testset "overflow of reftype is detected and doesn't corrupt levels" begin

test/13_arraycommon.jl

@@ -251,13 +251,14 @@ end
         x = CategoricalArray{Union{T, String}}(["Old", "Young", "Middle", "Young"])
         levels!(x, ["Young", "Middle", "Old"])
         ordered!(x, true)
+        y = CategoricalArray{Union{T, String}}(["X", "Z", "Y", "X"])
 
         for copyf! in (copy!, copyto!)
-            y = CategoricalArray{Union{T, String}}(["X", "Z", "Y", "X"])
-            @test copyf!(x, y) === x
-            @test x == y
-            @test levels(x) == ["Young", "Middle", "Old", "X", "Y", "Z"]
-            @test !isordered(x)
+            x2 = copy(x)
+            @test copyf!(x2, y) === x2
+            @test x2 == y
+            @test levels(x2) == ["Young", "Middle", "Old", "X", "Y", "Z"]
+            @test !isordered(x2)
         end
 
         x = CategoricalArray{Union{T, String}}(["Old", "Young", "Middle", "Young"])
@@ -506,6 +507,66 @@ end
         end
     end
 
+    @testset "assigning into array with empty levels uses orderedness of source" begin
+        # destination is marked as ordered when source is ordered
+        x = CategoricalArray{Union{T, String}}(["Old", "Young", "Middle", "Young"])
+        levels!(x, ["Young", "Middle", "Old"])
+        ordered!(x, true)
+
+        for copyf! in (copyto!, copy!)
+            y = CategoricalArray{Union{T, String}}(undef, 4)
+            copyf!(y, x)
+            @test isordered(y)
+            @test levels(y) == levels(x)
+            if T >: Missing
+                y = CategoricalArray{Union{T, String}}(fill(missing, 4))
+                copyf!(y, x)
+                @test isordered(y)
+                @test levels(y) == levels(x)
+            end
+        end
+
+        y = CategoricalArray{Union{T, String}}(undef, 4)
+        y[1] = x[1]
+        @test isordered(y)
+        @test levels(y) == levels(x)
+        if T >: Missing
+            y = CategoricalArray{Union{T, String}}(fill(missing, 4))
+            y[1] = x[1]
+            @test isordered(y)
+            @test levels(y) == levels(x)
+        end
+
+        # destination is marked as unordered when source is unordered
+        ordered!(x, false)
+
+        for copyf! in (copyto!, copy!)
+            y = CategoricalArray{Union{T, String}}(undef, 4)
+            ordered!(y, true)
+            copyf!(y, x)
+            @test !isordered(y)
+            @test levels(y) == levels(x)
+            if T >: Missing
+                y = CategoricalArray{Union{T, String}}(fill(missing, 4))
+                ordered!(y, true)
+                copyf!(y, x)
+                @test !isordered(y)
+                @test levels(y) == levels(x)
+            end
+        end
+
+        y = CategoricalArray{Union{T, String}}(undef, 4)
+        y[1] = x[1]
+        @test !isordered(y)
+        @test levels(y) == levels(x)
+        if T >: Missing
+            y = CategoricalArray{Union{T, String}}(fill(missing, 4))
+            y[1] = x[1]
+            @test !isordered(y)
+            @test levels(y) == levels(x)
+        end
+    end
+
     @testset "resize!()" begin
         x = CategoricalArray{Union{T, String}}(["Old", "Young", "Middle", "Young"])
         @test resize!(x, 3) === x