Revert "(Re-)define one for Colorant (#243)"

This reverts commit 306ed7d.
JuliaGraphics · May 15, 2022 · 444307d · 444307d
1 parent ea8b889
commit 444307d
Show file tree

Hide file tree

Showing 6 changed files with 41 additions and 75 deletions.
diff --git a/src/ColorTypes.jl b/src/ColorTypes.jl
@@ -6,7 +6,7 @@ using Base: @pure
 
 const Fractional = Union{AbstractFloat, FixedPoint}
 
-import Base: ==, <, isless, isapprox, isfinite, isinf, isnan, one, oneunit, zero,
+import Base: ==, <, isless, isapprox, isfinite, isinf, isnan, oneunit, zero,
              hash, eltype, length, real, convert, reinterpret, show
 using Random
 import Random: rand

diff --git a/src/error_hints.jl b/src/error_hints.jl
@@ -2,15 +2,22 @@ MathTypes{T,C} = Union{AbstractRGB{T},TransparentRGB{C,T},AbstractGray{T},Transp
 
 # provided by https://github.com/JuliaLang/julia/pull/35094
 function register_hints()
-    isdefined(Base, :Experimental) && isdefined(Base.Experimental, :register_error_hint) || return
-    Base.Experimental.register_error_hint(MethodError) do io, exc, argtypes, kwargs
-        # In theory we could list every function supported by ColorVectorSpace.
-        # This list of functions is far from comprehensive but may be enough to catch many users.
-        if exc.f in (+, -, *, /) && any(T->T<:MathTypes || T<:Type{<:MathTypes}, argtypes)
-            print(io, "\nMath on colors is deliberately undefined in ColorTypes, but see the ColorVectorSpace package.")
-        end
-        if (exc.f === *) && all(T->T<:MathTypes || T<:Type{<:MathTypes}, argtypes)
-            print(io, "\nYou may also need `⋅`, `⊙`, or `⊗`.")
+    if isdefined(Base, :Experimental) && isdefined(Base.Experimental, :register_error_hint)
+        Base.Experimental.register_error_hint(MethodError) do io, exc, argtypes, kwargs
+            if exc.f in (one,) && argtypes[1] <: Union{Type{<:AbstractRGB}, AbstractRGB}
+                print(io, "\nYou may need to `using ColorVectorSpace`.")
+            end
+            if exc.f in (ones,) && argtypes[1] <: Type{<:AbstractRGB}
+                print(io, "\nYou may need to `using ColorVectorSpace`.")
+            end
+            # In theory we could list every function supported by ColorVectorSpace.
+            # This list of functions is far from comprehensive but may be enough to catch many users.
+            if exc.f in (+, -, *, /) && any(T->T<:MathTypes || T<:Type{<:MathTypes}, argtypes)
+                print(io, "\nMath on colors is deliberately undefined in ColorTypes, but see the ColorVectorSpace package.")
+            end
+            if (exc.f === *) && all(T->T<:MathTypes || T<:Type{<:MathTypes}, argtypes)
+                print(io, "\nYou may also need `⋅`, `⊙`, or `⊗`.")
+            end
         end
     end
 end
diff --git a/src/traits.jl b/src/traits.jl
@@ -477,10 +477,6 @@ zero(::Type{C}) where {C<:ColorantN{4}} = C(0, 0, 0, 0)
 zero(::Type{C}) where {C<:ColorantN{5}} = C(0, 0, 0, 0, 0)
 zero(c::Colorant) = zero(typeof(c))
 
-oneunit(::Type{C}) where {C<:Colorant} = throw_oneunit_error(C)
-@noinline function throw_oneunit_error(@nospecialize(C))
-    throw(ArgumentError("`oneunit` for $C is not defined. Perhaps the meaning is not clear."))
-end
 oneunit(::Type{C}) where {C<:AbstractGray} = C(1)
 # It's not clear what `oneunit` means for most Color3s,
 # but for AbstractRGB, XYZ, and LMS, it's OK
@@ -490,8 +486,9 @@ oneunit(::Type{C}) where {C<:TransparentColor} = C(oneunit(color_type(C)))
 oneunit(::Type{C}) where {C<:Union{AGray, GrayA}} = C(1, 1) # workaround for inconsistent `color_type`
 oneunit(c::Colorant) = oneunit(typeof(c))
 
-one(::Type{C}) where {C<:Colorant} = one(eltype_default(C))
-one(::Type{C}) where {T, C<:Colorant{T}} = one(T)
-one(c::Colorant) = one(typeof(c))
+function Base.one(::Type{C}) where {C<:Gray}
+    Base.depwarn("one($C) will soon switch to returning 1; you might need to switch to `oneunit`", :one)
+    C(1)
+end
 
 Base.broadcastable(x::Colorant) = Ref(x)
diff --git a/test/error_hints.jl b/test/error_hints.jl
@@ -15,6 +15,21 @@ macro except_str(expr, err_type)
 end
 
 @testset "error hints" begin
+    @testset "ones" begin
+        for T in (RGB, RGB{N0f8})
+            err_str = @except_str one(T) MethodError
+            @test occursin(r"MethodError: no method matching one\(::Type\{RGB.*\}", err_str)
+            @test occursin("You may need to `using ColorVectorSpace`.", err_str)
+
+            err_str = @except_str one(T(1, 1, 1)) MethodError
+            @test occursin(r"MethodError: no method matching one\(::RGB\{.*\}", err_str)
+            @test occursin("You may need to `using ColorVectorSpace`.", err_str)
+
+            err_str = @except_str ones(T) MethodError
+            @test occursin(r"MethodError: no method matching one\(::Type\{RGB.*\}", err_str)
+            @test occursin("You may need to `using ColorVectorSpace`.", err_str)
+        end
+    end
     @testset "Math" begin
         gray = Gray(0.8)
         rgb = RGB{Float32}(1, 0, 0)

diff --git a/test/operations.jl b/test/operations.jl
@@ -252,29 +252,3 @@ end
     @test @inferred(mapreducec(x->!x, &, true, false))
     @test !@inferred(mapreducec(x->!x, &, false, false))
 end
-
-@testset "ones/zeros" begin
-    for C in (Gray, Gray{N0f8}, GrayA{Float32}, Gray24, AGray32,
-              RGB, RGB{N0f8}, RGBA{Float32}, RGB24, ARGB32)
-        for f in (ones, zeros)
-            if f === ones && C <: TransparentColor
-                @test_broken f(C, 3, 5) # issue #162
-                continue
-            end
-            mat = @inferred(f(C, 3, 5))
-            # note that the return type of `ones(RGB)` is `Array{RGB}`, not `Array{RGB{N0f8}}`
-            @test typeof(mat) === Matrix{C}
-            @test size(mat) == (3, 5)
-            @test mat[2, 3] === (f === ones ? oneunit(C) : zero(C))
-        end
-    end
-    @test_throws ColorTypes.ColorTypeResolutionError ones(HSV{Float32}, 3, 5)
-    # Although `XYZ` and `LMS` have the definition of `oneunit`,
-    # it is generally not equivalent to `Gray(1)`.
-    @test_throws ColorTypes.ColorTypeResolutionError ones(XYZ, 3, 5)
-    @test_throws ColorTypes.ColorTypeResolutionError ones(LMS{Float64}, 3, 5)
-
-    @test zeros(HSV{Float32}, 3, 5)[2, 3] === zero(HSV{Float32})
-    @test zeros(XYZ, 3, 5)[2, 3] === zero(XYZ)
-    @test zeros(LMS{Float64}, 3, 5)[2, 3] === zero(LMS{Float64})
-end
diff --git a/test/traits.jl b/test/traits.jl
@@ -632,108 +632,81 @@ end
     @test_throws MethodError ColorTypes.nan(Gray(1.0))
 end
 
-# see also ones/zeros test in "test/operations.jl"
 @testset "identities for Gray" begin
-    @test one(    Gray{N0f8}) === 1N0f8
     @test oneunit(Gray{N0f8}) === Gray{N0f8}(1)
     @test zero(   Gray{N0f8}) === Gray{N0f8}(0)
-    @test @inferred(one(    Gray)) === 1N0f8
     @test @inferred(oneunit(Gray)) === Gray{N0f8}(1)
     @test @inferred(zero(   Gray)) === Gray{N0f8}(0)
-    @test one(    Gray{Bool}) === true
     @test oneunit(Gray{Bool}) === Gray{Bool}(1)
     @test zero(   Gray{Bool}) === Gray{Bool}(0)
 
-    @test one(    AGray{N0f8}) === 1N0f8
     @test oneunit(AGray{N0f8}) === AGray{N0f8}(1, 1)
     @test zero(   AGray{N0f8}) === AGray{N0f8}(0, 0)
-    @test @inferred(one(    AGray)) === 1N0f8
     @test @inferred(oneunit(AGray)) === AGray{N0f8}(1, 1)
     @test @inferred(zero(   AGray)) === AGray{N0f8}(0, 0)
-    @test one(    GrayA{Float32}) === 1.0f0
     @test oneunit(GrayA{Float32}) === GrayA{Float32}(1, 1)
     @test zero(   GrayA{Float32}) === GrayA{Float32}(0, 0)
 
-    @test one(    Gray24) === 1N0f8
     @test oneunit(Gray24) === Gray24(1)
     @test zero(   Gray24) === Gray24(0)
-    @test one(   AGray32) === 1N0f8
     @test oneunit(AGray32) === AGray32(1, 1)
     @test zero(   AGray32) === AGray32(0, 0)
 
     g = Gray{Float32}(0.8)
-    @test one(    g) === 1.0f0
     @test oneunit(g) === Gray{Float32}(1)
     @test zero(   g) === Gray{Float32}(0)
 
     @test_broken one(Gray{Float32}) * g == g * one(Gray{Float32}) == g
 end
 
 @testset "identities for RGB" begin
-    @test one(    RGB{N0f8}) === 1N0f8
     @test oneunit(RGB{N0f8}) === RGB{N0f8}(1, 1, 1)
     @test zero(   RGB{N0f8}) === RGB{N0f8}(0, 0, 0)
-    @test @inferred(one(    RGB)) === 1N0f8
     @test @inferred(oneunit(RGB)) === RGB{N0f8}(1, 1, 1)
     @test @inferred(zero(   RGB)) === RGB{N0f8}(0, 0, 0)
 
-    @test one(    ARGB{N0f8}) === 1N0f8
     @test oneunit(ARGB{N0f8}) === ARGB{N0f8}(1, 1, 1, 1)
     @test zero(   ARGB{N0f8}) === ARGB{N0f8}(0, 0, 0, 0)
-    @test @inferred(one(    ARGB)) === 1N0f8
     @test @inferred(oneunit(ARGB)) === ARGB{N0f8}(1, 1, 1, 1)
     @test @inferred(zero(   ARGB)) === ARGB{N0f8}(0, 0, 0, 0)
-    @test one(    RGBA{Float32}) === 1.0f0
     @test oneunit(RGBA{Float32}) === RGBA{Float32}(1, 1, 1, 1)
     @test zero(   RGBA{Float32}) === RGBA{Float32}(0, 0, 0, 0)
 
-    @test one(    RGB24) === 1N0f8
     @test oneunit(RGB24) === RGB24(1, 1, 1)
     @test zero(   RGB24) === RGB24(0, 0, 0)
-    @test one(    ARGB32) === 1N0f8
     @test oneunit(ARGB32) === ARGB32(1, 1, 1, 1)
     @test zero(   ARGB32) === ARGB32(0, 0, 0, 0)
 
     c = RGB{Float32}(0.4, 0.5, 0.6)
-    @test one(    c) === 1.0f0
     @test oneunit(c) === RGB{Float32}(1, 1, 1)
     @test zero(   c) === RGB{Float32}(0, 0, 0)
 end
 
 @testset "identities for other colors" begin
-    @test one(    XYZ{Float16}) === Float16(1)
     @test oneunit(XYZ{Float16}) === XYZ{Float16}(1, 1, 1)
     @test zero(   XYZ{Float16}) === XYZ{Float16}(0, 0, 0)
 
-    @test @inferred(one(    LMS)) === 1.0f0
     @test @inferred(oneunit(LMS)) === LMS{Float32}(1, 1, 1)
     @test @inferred(zero(   LMS)) === LMS{Float32}(0, 0, 0)
 
-    @test one( HSV{Float32}) === 1.0f0
-    @test_throws ArgumentError oneunit(HSV{Float32})
+    @test_throws MethodError oneunit(HSV{Float32})
     @test zero(HSV{Float32}) === HSV{Float32}(0, 0, 0)
 
-    @test one( ALab{Float16}) === Float16(1)
-    @test_throws ArgumentError oneunit(ALab{Float16})
+    @test_throws MethodError oneunit(ALab{Float16})
     @test zero(ALab{Float16}) === ALab{Float16}(0, 0, 0, 0)
 
-    @test one( LCHuvA{Float64}) === 1.0
-    @test_throws ArgumentError oneunit(LCHuvA{Float64})
+    @test_throws MethodError oneunit(LCHuvA{Float64})
     @test zero(LCHuvA{Float64}) === LCHuvA{Float64}(0, 0, 0, 0)
 
-    @test one( C2{Float64}) === 1.0
-    @test_throws ArgumentError oneunit(C2{Float64})
+    @test_throws MethodError oneunit(C2{Float64})
     @test zero(C2{Float64}) === C2{Float64}(0, 0)
 
-    @test one( C4{Float64}) === 1.0
-    @test_throws ArgumentError oneunit(C4{Float64})
+    @test_throws MethodError oneunit(C4{Float64})
     @test zero(C4{Float64}) === C4{Float64}(0, 0, 0, 0)
 
-    @test one(    CMYK{N0f8}) === 1N0f8
     @test oneunit(CMYK{N0f8}) === CMYK{N0f8}(1, 1, 1, 1)
     @test zero(   CMYK{N0f8}) === CMYK{N0f8}(0, 0, 0, 0)
 
-    @test one(    ACMYK{N0f8}) === 1N0f8
     @test oneunit(ACMYK{N0f8}) === ACMYK{N0f8}(1, 1, 1, 1, 1)
     @test zero(   ACMYK{N0f8}) === ACMYK{N0f8}(0, 0, 0, 0, 0)
 end