search

JuliaPlots / VisualRegressionTests.jl

Automated integrated regression tests for graphics libraries
Other
29 stars 11 forks source link

Bump deps #34

Closed t-bltg closed 3 years ago

t-bltg commented 3 years ago

@mateuszbaran, any idea why the tests fails now, and how we can fix this ?

cc @johnnychen94 who might know ColorTypes or ColorVectorSpace better than me.

Macros: Error During Test at /home/runner/work/VisualRegressionTests.jl/VisualRegressionTests.jl/src/macros.jl:18
  Test threw exception
  Expression: test_images(testFilename, "PlotTest.png", popup = !istravis, tol = 0.02) |> success
  MethodError: no method matching +(::Float64, ::RGB{Float64})
  Math on colors is deliberately undefined in ColorTypes, but see the ColorVectorSpace package.
  Closest candidates are:
    +(::Any, ::Any, !Matched::Any, !Matched::Any...) at operators.jl:560
    +(::Union{Float16, Float32, Float64}, !Matched::BigFloat) at mpfr.jl:392
    +(!Matched::AbstractRGB, ::AbstractRGB) at /home/runner/.julia/packages/ColorVectorSpace/s7hKD/src/ColorVectorSpace.jl:262
    ...
  Stacktrace:
    [1] sumdiff
      @ ~/work/VisualRegressionTests.jl/VisualRegressionTests.jl/src/utils.jl:34 [inlined]
    [2] sad
      @ ~/work/VisualRegressionTests.jl/VisualRegressionTests.jl/src/utils.jl:26 [inlined]
    [3] blurdiff(A::Matrix{RGB{FixedPointNumbers.N0f8}}, B::Matrix{RGB{FixedPointNumbers.N0f8}}, sigma::Tuple{Int64, Int64})
      @ VisualRegressionTests ~/work/VisualRegressionTests.jl/VisualRegressionTests.jl/src/utils.jl:19
    [4] compare_images(testfn::String, reffn::String; sigma::Tuple{Int64, Int64}, tol::Float64)
      @ VisualRegressionTests ~/work/VisualRegressionTests.jl/VisualRegressionTests.jl/src/imgcomp.jl:15
    [5] test_images(testfn::String, reffn::String; popup::Bool, newfn::String, kw::Base.Iterators.Pairs{Symbol, Float64, Tuple{Symbol}, NamedTuple{(:tol,), Tuple{Float64}}})
      @ VisualRegressionTests ~/work/VisualRegressionTests.jl/VisualRegressionTests.jl/src/imgcomp.jl:31
    [6] macro expansion
      @ ~/work/VisualRegressionTests.jl/VisualRegressionTests.jl/src/macros.jl:18 [inlined]
    [7] macro expansion
      @ ~/work/VisualRegressionTests.jl/VisualRegressionTests.jl/test/runtests.jl:29 [inlined]
    [8] macro expansion
      @ /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.6/Test/src/Test.jl:1151 [inlined]
    [9] macro expansion
      @ ~/work/VisualRegressionTests.jl/VisualRegressionTests.jl/test/runtests.jl:27 [inlined]
   [10] macro expansion
      @ /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.6/Test/src/Test.jl:1151 [inlined]
   [11] top-level scope
      @ ~/work/VisualRegressionTests.jl/VisualRegressionTests.jl/test/runtests.jl:18
mateuszbaran commented 3 years ago

I would guess that you should replace abs with norm in https://github.com/JuliaPlots/VisualRegressionTests.jl/blob/cb6433496fd451572fafe560a80ef8db5e2739ef/src/utils.jl#L26 .

johnnychen94 commented 3 years ago

There are some hard landings here for the recent CVS development, one of the major breaking change is that abs and abs2 are undefined for RGB types https://github.com/JuliaGraphics/ColorVectorSpace.jl/pull/131. I would suggest drop all ColorTypes and ColorVectorSpace dependency, and use ImageCore@0.9 instead; ImageCore@0.9 reexports ColorTypes and also loads CVS 0.9.

To upgrade the codebase to CVS@0.9, one needs to define his own _abs method, https://github.com/JuliaGraphics/ColorVectorSpace.jl#abs-and-abs2, and then replace sumdiff(abs, A, B) with sumdiff(_abs, A, B).

t-bltg commented 3 years ago

There are some hard landings here for the recent CVS development, one of the major breaking change is that abs and abs2 are undefined for RGB types JuliaGraphics/ColorVectorSpace.jl#131. I would suggest drop all ColorTypes and ColorVectorSpace dependency, and use ImageCore@0.9 instead; ImageCore@0.9 reexports ColorTypes and also loads CVS 0.9.

To upgrade the codebase to CVS@0.9, one needs to define his own _abs method, https://github.com/JuliaGraphics/ColorVectorSpace.jl#abs-and-abs2, and then replace sumdiff(abs, A, B) with sumdiff(_abs, A, B).

Thanks for taking the time to answer, very clear.

johnnychen94 commented 3 years ago

Images dependency might be too large for this package; and Images isn't yet compatible with CVS 0.9 yet so you might get a fake fix by adding Images dependency.

t-bltg commented 3 years ago

Images dependency might be too large for this package; and Images isn't yet compatible with CVS 0.9 yet so you might get a fake fix by adding Images dependency.

The reason why I'm trying to add Images is that the implementation of maxabsfinite was failing here, should I copy the whole code for maxabsfinite of https://github.com/JuliaImages/Images.jl/blob/master/src/algorithms.jl here ?.

Is there a reason why these algorithms are not in ImageCore instead ?

It's cumbersome:

johnnychen94 commented 3 years ago

Ah okay, that now lives in ImageBase v0.1.3

ImageCore is designed to be a collection of traits and array types so we don't add many functions there.

johnnychen94 commented 3 years ago

See also https://github.com/JuliaImages/ImageBase.jl/issues/14

t-bltg commented 3 years ago

Ah okay, that now lives in ImageBase v0.1.3

Oh thanks for pointing ImageBase.

t-bltg commented 3 years ago

God, that took an awful number of runs to get fixed for an equivalent 2 lines change. I forgot how nasty colors and images can be, definitely not my expertise area. I'm glad people like @johnnychen94 eat this at beakfast :laughing: