Skip to content

Add complement migrated from Images.jl #144

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

Merged
merged 1 commit into from
Mar 4, 2021

Conversation

kimikage
Copy link
Collaborator

@kimikage kimikage commented Mar 4, 2021

This is the part of PR #125 about complement. (cf. JuliaImages/Images.jl#698, JuliaImages/Images.jl#879)

The differences from the original are as follows:

  • This exports complement (as suggested in Bug/move std to cvs #125 (comment)).
  • This drops the deprecation for array input.
    • It is the responsibility of Images.jl.
  • This modifies the test.

Images.jl does not yet support ColorVectorSpace v0.9, so it is possible to include this in v0.9, but I don't think that would be a good idea.

Also, oneunit(::AbstractGray) has been missing. 😕 This may be fixed in ColorTypes v0.11.

cc: @wizofe

@kimikage kimikage mentioned this pull request Mar 4, 2021
@codecov
Copy link

codecov bot commented Mar 4, 2021

Codecov Report

Merging #144 (2462bc0) into master (578fe82) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #144      +/-   ##
==========================================
+ Coverage   97.12%   97.15%   +0.02%     
==========================================
  Files           2        2              
  Lines         209      211       +2     
==========================================
+ Hits          203      205       +2     
  Misses          6        6              
Impacted Files Coverage Δ
src/ColorVectorSpace.jl 97.98% <100.00%> (+0.02%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 578fe82...fa63b18. Read the comment docs.

@timholy timholy merged commit 3072185 into JuliaGraphics:master Mar 4, 2021
@timholy
Copy link
Member

timholy commented Mar 4, 2021

Also, oneunit(::AbstractGray) has been missing. This may be fixed in ColorTypes v0.11.

To clarify for other readers, it works if you load ColorVectorSpace but the method is among several that needs to migrate to ColorTypes.

@kimikage
Copy link
Collaborator Author

kimikage commented Mar 4, 2021

To clarify for other readers, it works if you load ColorVectorSpace but the method is among several that needs to migrate to ColorTypes.

No, it does not work. ColorVectorSpace hasn't supported it for a long time. ColorTypes only supports Gray.

julia> complement(Gray24(0.2))
ERROR: MethodError: no method matching one(::Type{Gray24})

@kimikage kimikage deleted the complement branch March 4, 2021 10:55
@timholy
Copy link
Member

timholy commented Mar 4, 2021

julia> using ColorTypes

julia> c = oneunit(Gray{Float32})
Gray{Float32}(1.0f0)

julia> oneunit(c)
ERROR: MethodError: no method matching one(::Gray{Float32})
Closest candidates are:
  one(::Union{Type{T}, T}) where T<:AbstractString at strings/basic.jl:262
  one(::Union{Type{P}, P}) where P<:Dates.Period at /home/tim/src/julia-master/usr/share/julia/stdlib/v1.7/Dates/src/periods.jl:54
  one(::LinearAlgebra.UniformScaling{T}) where T at /home/tim/src/julia-master/usr/share/julia/stdlib/v1.7/LinearAlgebra/src/uniformscaling.jl:132
  ...
Stacktrace:
 [1] oneunit(x::Gray{Float32})
   @ Base ./number.jl:318
 [2] top-level scope
   @ REPL[3]:1

julia> using ColorVectorSpace

julia> oneunit(c)
Gray{Float32}(1.0f0)

@kimikage
Copy link
Collaborator Author

kimikage commented Mar 4, 2021

That is true, but it is also misleading. What is important is not the difference between instance and type, but between Gray and AbstractGray.

In any case, I don't think it is necessary to add new seeds of depreation now.

@timholy
Copy link
Member

timholy commented Mar 4, 2021

Well, not quite true. Probably 99% of people use Gray rather than, e.g., Gray24.

Comment on lines +486 to +487
@test_broken complement(Gray24(0.2)) === Gray24(0.8)
@test_broken complement(AGray32(0.2, 0.7)) === AGray32(0.8, 0.7)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, oneunit(::AbstractGray) has been missing. This may be fixed in ColorTypes v0.11.

johnnychen94 added a commit to JuliaImages/Images.jl that referenced this pull request Oct 31, 2021
`complement` is such a basic operation that we moved to lower-level package
ColorVectorSpace in PR
JuliaGraphics/ColorVectorSpace.jl#144

This requires ColorVectorSpace v0.9.2. Since we indirectly get CVS via
ImageCore, we need to set a compatible ImageCore version that requires CVS >=
v0.9.2. For this reason, this commit sets ImageCore compatibility to v0.9.3 so
that we get CVS at least v0.9.7
johnnychen94 added a commit to johnnychen94/Images.jl that referenced this pull request May 21, 2022
`complement` is such a basic operation that we moved to lower-level package
ColorVectorSpace in PR
JuliaGraphics/ColorVectorSpace.jl#144

This requires ColorVectorSpace v0.9.2. Since we indirectly get CVS via
ImageCore, we need to set a compatible ImageCore version that requires CVS >=
v0.9.2. For this reason, this commit sets ImageCore compatibility to v0.9.3 so
that we get CVS at least v0.9.7
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