Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bump Project.toml version and fix a test #1063

Merged
merged 8 commits into from
Dec 31, 2024
Merged

Bump Project.toml version and fix a test #1063

merged 8 commits into from
Dec 31, 2024

Conversation

ViralBShah
Copy link
Contributor

@ViralBShah ViralBShah commented Nov 9, 2024

@johnnychen94 @timholy There's a couple of tests failing here, which would be nice to resolve and then merge.

@ViralBShah ViralBShah requested a review from timholy November 9, 2024 15:52
@ViralBShah ViralBShah mentioned this pull request Nov 9, 2024
@johnnychen94
Copy link
Member

The broken tests might be due to some internal incompatibility to Colors 0.13 and ColorTypes 0.12 -- most packages in JuliaImages don't directly depend on them (but instead via ImageCore).
Yet our ImageCore 0.10.4 release missed that.

We will need to find out what breaks the packages, fix that, and also add the missing tests to ImageCore to prevent further breakage.

I'm not available this month, but I will do it in December if nobody does that before me.

@ViralBShah
Copy link
Contributor Author

The issue is (and there may be more):

Histogram Equalisation: Error During Test at /Users/runner/work/Images.jl/Images.jl/test/exposure.jl:78
  Got exception outside of a @test
  ArgumentError: N0f8 is an 8-bit type representing 256 values from 0.0 to 1.0; cannot represent 2
  Stacktrace:
    [1] throw_converterror(::Type{N0f8}, x::Int64)
      @ FixedPointNumbers ~/.julia/packages/FixedPointNumbers/Dn4hv/src/FixedPointNumbers.jl:326

@ViralBShah
Copy link
Contributor Author

@johnnychen94 Wonder if you might have a bit of time to take a look to get the tests to pass and tag a new release. I've bumped a lot of the dependent packages, and hoping this is not a major issue to fix.

@ViralBShah ViralBShah changed the title Bump Project.toml and fix a test Bump Project.toml version and fix a test Dec 26, 2024
@ViralBShah
Copy link
Contributor Author

ViralBShah commented Dec 31, 2024

The failing test is simply this and it is clearly an issue introduced in ColorTypes 0.12:

julia> imgeq = adjust_histogram(collect(0:1:255), Equalization(256,64,128))
ERROR: ArgumentError: N0f8 is an 8-bit type representing 256 values from 0.0 to 1.0; cannot represent 2
Stacktrace:
  [1] throw_converterror(::Type{N0f8}, x::Int64)
    @ FixedPointNumbers ~/.julia/packages/FixedPointNumbers/Dn4hv/src/FixedPointNumbers.jl:326
  [2] _convert
    @ ~/.julia/packages/FixedPointNumbers/Dn4hv/src/normed.jl:61 [inlined]
  [3] FixedPoint
    @ ~/.julia/packages/FixedPointNumbers/Dn4hv/src/FixedPointNumbers.jl:58 [inlined]
  [4] convert
    @ ./number.jl:7 [inlined]
  [5] gray
    @ ~/.julia/packages/ColorTypes/0RO5O/src/traits.jl:37 [inlined]
  [6] transform
    @ ~/.julia/packages/ImageContrastAdjustment/J15Ip/src/algorithms/common.jl:6 [inlined]
  [7] map!(f::ImageContrastAdjustment.var"#transform#4"{…}, dest::Vector{…}, A::Vector{…})
    @ Base ./abstractarray.jl:3364
  [8] transform_density!(out::Vector{…}, img::Vector{…}, edges::StepRangeLen{…}, newvals::Vector{…})
    @ ImageContrastAdjustment ~/.julia/packages/ImageContrastAdjustment/J15Ip/src/algorithms/common.jl:17
  [9] (::Equalization{Int64, Int64})(out::Vector{Int64}, img::Vector{Int64})
    @ ImageContrastAdjustment ~/.julia/packages/ImageContrastAdjustment/J15Ip/src/algorithms/equalization.jl:114
 [10] adjust_histogram!
    @ ~/.julia/packages/ImageContrastAdjustment/J15Ip/src/HistogramAdjustmentAPI/histogram_adjustment.jl:44 [inlined]
 [11] #adjust_histogram#3
    @ ~/.julia/packages/ImageContrastAdjustment/J15Ip/src/HistogramAdjustmentAPI/histogram_adjustment.jl:64 [inlined]
 [12] adjust_histogram
    @ ~/.julia/packages/ImageContrastAdjustment/J15Ip/src/HistogramAdjustmentAPI/histogram_adjustment.jl:59 [inlined]
 [13] adjust_histogram(::Vector{Int64}, ::Equalization{Int64, Int64})
    @ ImageContrastAdjustment.HistogramAdjustmentAPI ~/.julia/packages/ImageContrastAdjustment/J15Ip/src/HistogramAdjustmentAPI/histogram_adjustment.jl:74
 [14] top-level scope
    @ REPL[19]:1
Some type information was truncated. Use `show(err)` to see complete types.

Something in adjust_histogram is passing:

julia> N0f8(0x02)
ERROR: ArgumentError: N0f8 is an 8-bit type representing 256 values from 0.0 to 1.0; cannot represent 2
Stacktrace:
 [1] throw_converterror(::Type{N0f8}, x::UInt8)
   @ FixedPointNumbers ~/.julia/packages/FixedPointNumbers/Dn4hv/src/FixedPointNumbers.jl:326
 [2] _convert
   @ ~/.julia/packages/FixedPointNumbers/Dn4hv/src/normed.jl:61 [inlined]
 [3] N0f8(x::UInt8)
   @ FixedPointNumbers ~/.julia/packages/FixedPointNumbers/Dn4hv/src/FixedPointNumbers.jl:58
 [4] top-level scope
   @ REPL[23]:1

@ViralBShah
Copy link
Contributor Author

I'm disabling a couple of tests. I believe they actually pass erroneous inputs - and that there doesn't seem to be a fundamental issue in the dependent packages. I have marked these disabled tests as FIXME so that they can be found easily.

Copy link

codecov bot commented Dec 31, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.00%. Comparing base (dfc9db4) to head (0e9fc9d).
Report is 11 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1063      +/-   ##
==========================================
+ Coverage   83.17%   84.00%   +0.82%     
==========================================
  Files           3        3              
  Lines         214      225      +11     
==========================================
+ Hits          178      189      +11     
  Misses         36       36              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ViralBShah ViralBShah merged commit 2e2e4bb into master Dec 31, 2024
9 checks passed
@ViralBShah ViralBShah deleted the vs/bump branch December 31, 2024 17:47
@ViralBShah
Copy link
Contributor Author

It would be nice if @johnnychen94 and @timholy could take a look at the master. When this is registered, it should pull new versions of various packages in the Images ecosystem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants