From 46cb73336477ed8405bdab59b059919b602c2bc2 Mon Sep 17 00:00:00 2001 From: Tim Holy Date: Sat, 4 Sep 2021 06:56:58 -0500 Subject: [PATCH] Support abs2 via `Future` To smoothly transition between the old `abs2` return value and the new one (which differs by a factor of 3 for RGB), this introduces an internal module `ColorVectorSpace.Future` permitting users to transition immediately to the new behavior. `Base.abs2` will return the old value with a depwarn; users can switch immediately to `ColorVectorSpace.Future.abs2` to avoid it. --- README.md | 39 +++++++++++++++++++++++++++++---------- src/ColorVectorSpace.jl | 36 ++++++++++++++++++++++++++++++++++++ test/runtests.jl | 12 ++++++++++-- 3 files changed, 75 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 09422b6..1fd355f 100644 --- a/README.md +++ b/README.md @@ -38,11 +38,12 @@ represents the "RGB vector space" version. This package also defines `norm(c)` for RGB and grayscale colors. This makes these color spaces [normed vector spaces](https://en.wikipedia.org/wiki/Normed_vector_space). -Note that `norm` has been designed to satisfy equivalence of grayscale and RGB representations: if +Note that `norm` has been designed to satisfy **equivalence** of grayscale and RGB representations: if `x` is a scalar, then `norm(x) == norm(Gray(x)) == norm(RGB(x, x, x))`. Effectively, there's a division-by-3 in the `norm(::RGB)` case compared to the Euclidean interpretation of the RGB vector space. Equivalence is an important principle for the Colors ecosystem, and violations should be reported as likely bugs. +One violation is `abs2`; see the section below for more detail. ## Usage @@ -117,15 +118,33 @@ The corresponding `stdmult` computes standard deviation. To begin with, there is no general and straightforward definition of the absolute value of a vector. -There are roughly two possible definitions of `abs`/`abs2`: as a channel-wise +There are two reasonably intuitive definitions of `abs`/`abs2`: as a channel-wise operator or as a function which returns a real number based on the norm. For the latter, there are also variations in the definition of norm. -In ColorVectorSpace v0.9 and later, `abs` is defined as a channel-wise operator -and `abs2` is undefined. -The following are alternatives for the definitions in ColorVectorSpace v0.8 and -earlier. -```julia -_abs(c) = mapreducec(v->abs(float(v)), +, 0, c) -_abs2(c) = mapreducec(v->float(v)^2, +, 0, c) -``` +In ColorVectorSpace v0.9 and later, `abs` is defined as a channel-wise operator. +`abs2` returns a real-valued scalar. In previous versions of ColorVectorSpace, +for `g = Gray(0.3)`, ColorVectorSpace returned different values for `abs2(g)` and +`abs2(RGB(g))`. This breaks the equivalence of `g` and `RGB(g)`. +This behavior is retained, with a deprecation warning, starting with +ColorVectorSpace 0.9.6. + +**In the future**, `abs2` will be defined as `abs2(c) == c⋅c ≈ norm(c)^2`. +This effectively divides the old result by 3; code that imposes thresholds +on `abs2(c)` may need to be updated. +You can obtain that behavior now--and circumvent the deprecation warning-- +by using `ColorVectorSpace.Future.abs2(c)`. + +We anticipate the following transition schedule: + +- Sept 20, 2021: release ColorVectorSpace 0.9.6 with both `abs2` and `Future.abs2`. + `abs2` will have a "quiet" deprecation warning (visible with `--depwarn=yes` + or when running `Pkg.test`) +- Jan 1, 2022: make the deprecation warning "noisy" (cannot be turned off). + This is designed to catch user-level scripts that may need to update thresholds + or other constants. +- *Apr 1, 2022: transition `abs2` to the new definition + and make `Future.abs2` give a "noisy" depwarn to revert to regular `abs2`. +- *July 1, 2022: remove `Future.abs2`. + +The two marked with `*` denote breaking releases. diff --git a/src/ColorVectorSpace.jl b/src/ColorVectorSpace.jl index a0eb0b3..cc483c9 100644 --- a/src/ColorVectorSpace.jl +++ b/src/ColorVectorSpace.jl @@ -41,6 +41,10 @@ if !hasmethod(zero, (Type{TransparentGray},)) zero(p::Colorant) = zero(typeof(p)) end +if !hasmethod(one, (Type{Gray24},)) + Base.one(::Type{Gray24}) = Gray24(1) +end + if !hasmethod(one, (Type{TransparentGray},)) # specification change is planned for ColorTypes v0.12 Base.one(::Type{C}) where {C<:TransparentGray} = C(1,1) Base.one(::Type{C}) where {C<:AbstractRGB} = C(1,1,1) @@ -481,6 +485,38 @@ end Base.length(r::StepRange{<:AbstractGray,<:AbstractGray}) = length(StepRange(gray(r.start), gray(r.step), gray(r.stop))) Base.length(r::StepRange{<:AbstractGray}) = length(StepRange(gray(r.start), r.step, gray(r.stop))) +Base.abs2(g::AbstractGray) = abs2(gray(g)) +function Base.abs2(c::AbstractRGB) + Base.depwarn(""" + The return value of `abs2` will change to ensure that `abs2(g::Gray) ≈ abs2(RGB(g::Gray))`. + For `RGB` colors, this results in dividing the previous output by 3. + + To avoid this warning, use `ColorVectorSpace.Future.abs2` instead of `abs2`; currently, + `abs2` returns the old value (for compatibility), and `ColorVectorSpace.Future.abs2` returns the new value. + When making this change, you may also need to adjust constants like color-difference thresholds + to compensate for the change in the returned value. + + If you are getting this from `var`, use `varmult` instead. + """, :abs2) + return mapreducec(v->float(v)^2, +, zero(eltype(c)), c) +end + +module Future +using ..ColorTypes +using ..ColorVectorSpace: ⋅, dot +""" + ColorVectorSpace.Future.abs2(c) + +Return a scalar "squared magnitude" for color types. For RGB and gray, this is just the mean-square +channelwise intensity. + +Compatibility note: this gives a different result from `Base.abs2(c)`, but eventually `Base.abs2` will switch +to the definition used here. Using `ColorVectorSpace.Future.abs2` thus future-proofs your code. +For more information about the transition, see ColorVectorSpace's README. +""" +abs2(c::Union{Real,AbstractGray,AbstractRGB}) = c ⋅ c +end + function __init__() if isdefined(Base, :Experimental) && isdefined(Base.Experimental, :register_error_hint) Base.Experimental.register_error_hint(MethodError) do io, exc, argtypes, kwargs diff --git a/test/runtests.jl b/test/runtests.jl index a613250..68c6f6a 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -137,7 +137,7 @@ ColorTypes.comp2(c::RGBA32) = alpha(c) @test_throws MethodError cf ÷ cf @test cf + 0.1 === 0.1 + cf === Gray(Float64(0.1f0) + 0.1) @test cf64 - 0.1f0 === -(0.1f0 - cf64) === Gray( 0.2 - Float64(0.1f0)) - @test_throws MethodError abs2(ccmp) + @test ColorVectorSpace.Future.abs2(ccmp) === ColorVectorSpace.Future.abs2(gray(ccmp)) @test norm(cf) == norm(cf, 2) == norm(gray(cf)) @test norm(cf, 1) == norm(gray(cf), 1) @test norm(cf, Inf) == norm(gray(cf), Inf) @@ -413,7 +413,10 @@ ColorTypes.comp2(c::RGBA32) = alpha(c) @test isinf(RGB(1, Inf, 0.5)) @test !isnan(RGB(1, Inf, 0.5)) @test abs(RGB(0.1,0.2,0.3)) == RGB(0.1,0.2,0.3) - @test_throws MethodError abs2(RGB(0.1,0.2,0.3)) + cv = RGB(0.1,0.2,0.3) + @test ColorVectorSpace.Future.abs2(cv) == cv ⋅ cv + @test_logs (:warn, r"change to ensure") abs2(cv) > 2*ColorVectorSpace.Future.abs2(cv) + @test ColorVectorSpace.Future.abs2(cv) ≈ norm(cv)^2 @test_throws MethodError sum(abs2, RGB(0.1,0.2,0.3)) @test norm(RGB(0.1,0.2,0.3)) ≈ sqrt(0.14)/sqrt(3) @@ -789,6 +792,7 @@ ColorTypes.comp2(c::RGBA32) = alpha(c) @test norm(x, p) == norm(g, p) ≈ norm(c, p) end @test dot(x, x) == dot(g, g) ≈ dot(c, c) + @test abs2(x) == abs2(g) ≈ ColorVectorSpace.Future.abs2(c) @test_throws MethodError mapreduce(x->x^2, +, c) # this risks breaking equivalence & noniterability end @@ -816,6 +820,10 @@ ColorTypes.comp2(c::RGBA32) = alpha(c) sv2 = mapc(sqrt, v2) @test varmult(⊙, cs, dims=1) ≈ [v2 v2] @test stdmult(⊙, cs, dims=1) ≈ [sv2 sv2] + + # When ColorVectorSpace.Future.abs2 becomes the default, delete the `@test_logs` + # and change the `@test_broken` to `@test` + @test_logs (:warn, r"will change to ensure") match_mode=:any @test_broken var(cs) == varmult(⋅, cs) end @testset "copy" begin