Create a Complement type

[mkitti]

This adds a Complement{C,T,N} type.

The complement is to be interpreted as a lazy complement.

Math on Complement affects its underlying Colorant.

[mkitti]

I will probably move this into its file that is included near the top of the file.

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 0% with 46 lines in your changes are missing coverage. Please review.

Project coverage is 77.63%. Comparing base (4d8eaaa) to head (7245c8b).

:exclamation: Current head 7245c8b differs from pull request most recent head 070de6b. Consider uploading reports for the commit 070de6b to get more accurate results

Files	Patch %	Lines
src/ColorVectorSpace.jl	0.00%	46 Missing :warning:

Additional details and impacted files

@@             Coverage Diff             @@
##           master     #189       +/-   ##
===========================================
- Coverage   91.04%   77.63%   -13.41%     
===========================================
  Files           3        3               
  Lines         268      313       +45     
===========================================
- Hits          244      243        -1     
- Misses         24       70       +46     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[mkitti]

@tlnagy @timholy could you review?

[tlnagy]

I don't have a ton of experience with ColorVectorSpace, but this seems like a reasonable implementation.

[mkitti]

I'm tempted to drop the math part from line 315 down

[tlnagy]

Bump, this would be nice to have merged for https://github.com/tlnagy/TiffImages.jl/pull/134

[mkitti]

@tlnagy what are the minimum features that you need?

[tlnagy]

I think it would be nice for Complemented values to be realized anytime you look at them. So it would be lazy until you accessed the values (like for display)

julia> res = Gray{N0f8}.(Matrix(I, 5, 5));

julia> Complement.(res) # doesn't work

TypeError: in typeassert, expected Complement{N0f8, 1, Gray{N0f8}}, got a value of type Gray{N0f8}

Stacktrace:
 [1] setindex!
   @ ./array.jl:971 [inlined]
 [2] setindex!
   @ ./multidimensional.jl:670 [inlined]
 [3] macro expansion
   @ ./broadcast.jl:974 [inlined]
 [4] macro expansion
   @ ./simdloop.jl:77 [inlined]
 [5] copyto!
   @ ./broadcast.jl:973 [inlined]
 [6] copyto!
   @ ./broadcast.jl:926 [inlined]
 [7] copy
   @ ./broadcast.jl:898 [inlined]
 [8] materialize(bc::Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{2}, Nothing, Type{Complement}, Tuple{Matrix{Gray{N0f8}}}})
   @ Base.Broadcast ./broadcast.jl:873
 [9] top-level scope
   @ In[12]:1

julia> convert.(Gray{N0f8}, Complement.(res)) # does work, but clunky

For TiffImages the idea would be to return an <: AbstractArray{Complement{Gray{N0f8}}, 3} and the users wouldn't even be able to tell that the complementing is happening when they access the data. It should just happen behind the scenes and behave identically to the standard <: AbstractArray{Gray{N0f8}, 3} containers.

[mkitti]

Wait... why doesn't Complement.(res) work?

julia> using ColorVectorSpace, Colors, FixedPointNumbers, LinearAlgebra

julia> res = Gray{N0f8}.(Matrix(I, 5, 5));

julia> Complement.(res)
5×5 Array{Complement{N0f8},2} with eltype Complement{N0f8, 1, Gray{N0f8}}:
 Complement(Gray{N0f8}(1.0))  Complement(Gray{N0f8}(0.0))  …  Complement(Gray{N0f8}(0.0))  Complement(Gray{N0f8}(0.0))
 Complement(Gray{N0f8}(0.0))  Complement(Gray{N0f8}(1.0))     Complement(Gray{N0f8}(0.0))  Complement(Gray{N0f8}(0.0))
 Complement(Gray{N0f8}(0.0))  Complement(Gray{N0f8}(0.0))     Complement(Gray{N0f8}(0.0))  Complement(Gray{N0f8}(0.0))
 Complement(Gray{N0f8}(0.0))  Complement(Gray{N0f8}(0.0))     Complement(Gray{N0f8}(1.0))  Complement(Gray{N0f8}(0.0))
 Complement(Gray{N0f8}(0.0))  Complement(Gray{N0f8}(0.0))     Complement(Gray{N0f8}(0.0))  Complement(Gray{N0f8}(1.0))

I see that it should display, but it should "work".

[tlnagy]

I had ImageShow loaded in a Jupyter notebook, maybe that's why?

IMO the lazy complement should be realized on display.

[mkitti]

I think the problem is that Complement is a Colorant rather than a Color. Perhaps it should be a Color.

[mkitti]

I made Complement <: Color, so now it renders in VS Code. I'm not sure if that is the best design at the moment. Maybe I should write particular display for Complement?

[tlnagy]

Would it be possible to write a display for Complement that just punts to the correct display based on the wrapped type? i.e. Complement{T, N, C <: Colorant{T,N}} realizes the transform and then punts to the display for Colorant{T, N} etc

[mkitti]

I believe so.

My understanding is that this is implemented in Colors.jl:

https://github.com/JuliaGraphics/Colors.jl/blob/master/src%2Fdisplay.jl

It looks like we just need to overload show for Complement. I think the implementation could be simple in that we would do the conversion to C, the underlying Colorant, and then invoke its show method.

[mkitti]

I have an implementation of display now.

julia> using ColorVectorSpace, Colors, FixedPointNumbers, LinearAlgebra

julia> res = rand(RGB{Float64}, 10, 10)

julia> c_res = reinterpret(Complement, res) # less allocations

juila> using TestImages

julia> cameraman = testimage("cameraman.tif")

juila> c_cameraman = reinterpret(Complement, cameraman) # no allocation complement

julia> ca_cameraman = ComplementArray(c_cameraman) # no allocation, now of element type Gray{N0f8}

julia> [cameraman ca_cameraman]

[mkitti]

I'm getting tempted to implement this in its own package. It seems slightly out of place here and we might be able to iterate on it faster if it were independent.

[tlnagy]

Looks great! There's a slight issue though with display for larger images:

using ColorVectorSpace
using Colors
using FixedPointNumbers
using ImageShow
using LinearAlgebra

res = Gray{N0f8}.(Matrix(I, 500, 500)) # no warning
#snip

@time reinterpret(Complement, res) # works but with warning
#snip
┌ Warning: Output swatches are reduced due to the large size (500×500).
│ Load the ImageShow package for large images.
└ @ Colors ~/.julia/packages/Colors/mIuXl/src/display.jl:15

This happens despite ImageShow being loaded and this warning doesn't happen for non-Complemented images.

EDIT: wrapping with ComplementArray fixes this:

ComplementArray(reinterpret(Complement, res)) # no warning

[mkitti]

Should we push to get this merged now or should I break out into a package to iterate on this independently?

[tlnagy]

Given that complement lives here, I think it makes sense to have the lazy version here as well. But I'm willing to defer to other folks who know the Colors ecosystem better. Would be nice to have merged either way.

[timholy]

Sorry, I'll try to get this a review by next Monday. I got started and remembered getting worried about something fundamental, but that was long enough ago that I've forgotten.

[mkitti]

There's been considerable changes since you last took a look, Tim.

1. Complement subtypes Color instead of Colorant
2. I changed the order of parameters from {T,N,C} to {C,T,N}
3. I dropped arithmetic on Complement
4. Accessing the components of Complement returns the complement of the parent Colorant.

[mkitti]

Implementation of Base.reinterpret(::Type{Complement}, array::AbstractArray{C}) where {T, N, C <: Colorant{T,N}} may be an abuse because Complement by itself is not fully specified with parameters. However, it is inferrable that we mean Complement{C,T,N}

[tlnagy]

Bump @mkitti, this would be great to have!

[mkitti]

I agree. I just have to find some time to get back around to it. Feel free to beat me to it.

[mkitti]

@timholy and @tlnagy I have written tests.

[mkitti]

Thanks for adding the tests and other fixes! It's looking good, but I've left a few other notes about things that seem likely to need fixing.

In cases where you're unsure how something should behave, I tend to expect that mappedarray(complement, a) should be a reasonable guide for how ComplementArray should behave. IF, that is, I understand why it's here at all.

• mappedarray(complement, a) is a good choice if you have an array of C <: Colorant and want an AbstractArray{C} that has an element type of C.
• ComplementArray is for when you have a AbstractArray{Complement} and want a AbstractArray{C} where C <: Colorant. Now that you mention mappedarray, it could probably just be a mappedarray(parent, a).

[kimikage]

it could probably just be a mappedarray(parent, a).

or more symmetrically, mappedarray(complement, a)? (In short, we don't care if it supports Base.parent() or not.)

[kimikage]

My main concern is operation and support in the downstream packages.

julia> Complement(RGB(1,1,0)) isa AbstractRGB
false

It is not very pleasant to have to wrap AbstractRGB (or AbstactGray) with Union, in order to support Complement. One ad hoc solution is to define ComplementRGB{T} <: AbstractRGB{T}, etc., respectively.😑

One idea is to push such an ugly hack into a separate package. cf. https://github.com/kimikage/StorageColorTypes.jl

Edit: In other words, my real concern is whether to implement InvertedGray/ComplementGray in ColorTypes.jl at the start point of the discussion. xref: https://github.com/JuliaGraphics/ColorTypes.jl/issues/295#issuecomment-2101995601

[kimikage]

For reference, AbstractGray will be changed from an alias to an abstract type (sometime in the future). cf. https://github.com/JuliaGraphics/ColorTypes.jl/pull/252 Ironically, the change is to address this issue properly. (https://github.com/JuliaGraphics/ColorTypes.jl/issues/230)

[timholy]

The other option is to convert AbstractRGB into a trait. That's feasible but a little harder to work with (given that traits do not have a standard convenient interface) than leaving it as an abstract type.