The first breaking abs2 step (#188)

This switches `abs2` to be consistent with `ColorVectorSpace.Future.abs2`.

Co-authored-by: t-bltg <[email protected]>

.github/workflows/UnitTest.yml

@@ -4,10 +4,13 @@ on:
   create:
     tags:
   push:
-    branches:
-      - master
+    branches: [master]
   pull_request:
 
+concurrency: 
+  group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }}
+  cancel-in-progress: true
+
 jobs:
   test:
     runs-on: ${{ matrix.os }}
@@ -29,28 +32,13 @@ jobs:
             arch: x86
 
     steps:
-      - uses: actions/checkout@v2
-      - name: "Set up Julia"
-        uses: julia-actions/setup-julia@v1
+      - uses: actions/checkout@v3
+      - uses: julia-actions/setup-julia@latest
         with:
           version: ${{ matrix.julia-version }}
-
-      - name: Cache artifacts
-        uses: actions/cache@v1
-        env:
-          cache-name: cache-artifacts
-        with:
-          path: ~/.julia/artifacts 
-          key: ${{ runner.os }}-test-${{ env.cache-name }}-${{ hashFiles('**/Project.toml') }}
-          restore-keys: |
-            ${{ runner.os }}-test-${{ env.cache-name }}-
-            ${{ runner.os }}-test-
-            ${{ runner.os }}-
-
-      - name: "Unit Test"
-        uses: julia-actions/julia-runtest@master
-
-      - uses: julia-actions/julia-processcoverage@v1
-      - uses: codecov/codecov-action@v1
+      - uses: julia-actions/cache@v1
+      - uses: julia-actions/julia-runtest@latest
+      - uses: julia-actions/julia-processcoverage@latest
+      - uses: codecov/codecov-action@v3
         with:
           file: lcov.info

Project.toml

@@ -1,6 +1,6 @@
 name = "ColorVectorSpace"
 uuid = "c3611d14-8923-5661-9e6a-0046d554d3a4"
-version = "0.9.10"
+version = "0.10.0"
 
 [deps]
 ColorTypes = "3da002f7-5984-5a60-b8a6-cbb66c0b333f"

README.md

@@ -129,12 +129,6 @@ for `g = Gray(0.3)`, ColorVectorSpace returned different values for `abs2(g)` an
 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 21, 2021: release ColorVectorSpace 0.9.7 with both `abs2` and `Future.abs2`.
@@ -143,8 +137,8 @@ We anticipate the following transition schedule:
 - May 19, 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.
-- *July 1, 2022: transition `abs2` to the new definition
+- *July 20, 2023: transition `abs2` to the new definition
   and make `Future.abs2` give a "noisy" depwarn to revert to regular `abs2`.
-- *Dec 1, 2022: remove `Future.abs2`.
+- *Dec 1, 2023: remove `Future.abs2`.
 
-The two marked with `*` denote breaking releases.
+The line marked with `*` denotes a breaking release.

src/ColorVectorSpace.jl

@@ -26,16 +26,12 @@ import LinearAlgebra: norm, ⋅, dot, promote_leaf_eltypes  # norm1, norm2, norm
 using Statistics
 import Statistics: middle # and `_mean_promote`
 
+isdefined(Base, :get_extension) || using Requires
+
 export RGBRGB, complement, nan, dotc, dot, ⋅, hadamard, ⊙, tensor, ⊗, norm, varmult, stdmult
 
 MathTypes{T,C<:Union{AbstractGray{T},AbstractRGB{T}}} = Union{C,TransparentColor{C,T}}
 
-if Base.VERSION >= v"1.5"
-    @inline _depwarn(msg, funcsym; force=false) = Base.depwarn(msg, funcsym; force=force)
-else
-    @inline _depwarn(msg, funcsym; force=false) = Base.depwarn(msg, funcsym)
-end
-
 ## Version compatibility with ColorTypes
 ### TODO: Remove the definitons other than `one` when dropping ColorTypes v0.10 support
 
@@ -479,40 +475,32 @@ 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)
-    _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
+Base.abs2(c::Union{AbstractGray,AbstractRGB}) = c ⋅ c
 
 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.
+    using ..ColorTypes
+    using ..ColorVectorSpace: ⋅
+    """
+        ColorVectorSpace.Future.abs2(c)
+    Return a scalar "squared magnitude" for color types. For RGB and gray, this is just the mean-square
+    channelwise intensity.
+    """
+
+    if VERSION >= v"1.5"
+        @inline _depwarn(msg, funcsym; force=false) = Base.depwarn(msg, funcsym; force=force)
+    else
+        @inline _depwarn(msg, funcsym; force=false) = Base.depwarn(msg, funcsym)
+    end
 
-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
+    function abs2(c::Union{Real,AbstractGray,AbstractRGB})
+        _depwarn("""
+            The return value of `abs2` is now consistent with `ColorVectorSpace.Future.abs2`.
+            `ColorVectorSpace.Future.abs2` will be removed in the future, please switch to `abs2`.""", :abs2
+        )
+        c ⋅ c
+    end
 end
 
-isdefined(Base, :get_extension) || using Requires
-
 function __init__()
     if isdefined(Base, :Experimental) && isdefined(Base.Experimental, :register_error_hint)
         Base.Experimental.register_error_hint(MethodError) do io, exc, argtypes, kwargs

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 ColorVectorSpace.Future.abs2(ccmp) === ColorVectorSpace.Future.abs2(gray(ccmp))
+        @test abs2(ccmp) === 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)
@@ -430,8 +430,9 @@ ColorTypes.comp2(c::RGBA32) = alpha(c)
         @test abs(RGB(0.1,0.2,0.3)) == 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_logs (:warn, r"is now consistent") ColorVectorSpace.Future.abs2(cv)
+        @test abs2(cv) == cv ⋅ cv
+        @test 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)
 
@@ -807,7 +808,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 abs2(x) ≈ abs2(g) ≈ abs2(c)
         @test_throws MethodError mapreduce(x->x^2, +, c)   # this risks breaking equivalence & noniterability
     end
 
@@ -835,10 +836,7 @@ 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)
+        @test var(cs) == varmult(⋅, cs)
     end
 
     @testset "copy" begin