JuliaGraphics / ColorTypes.jl

Basic color definitions and traits
Other
77 stars 35 forks source link

Add `zero` & `oneunit` for more types #238

Closed kimikage closed 3 years ago

kimikage commented 3 years ago

This is a part of PR #186.

This moves the zero and oneunit for some of grays and rgbs that were defined in ColorVectorSpace to ColorTypes. (That is, this is a breaking change.) For zero, a color with all components zero is considered reasonable as an additive identity in most color spaces. On the other hand, it is difficult to define oneunit in general. Therefore, oneunit is defined only for specific color spaces.

This does not change the definition of one.

Fixes #219

codecov[bot] commented 3 years ago

Codecov Report

Merging #238 (138e229) into master (7f1d47c) will increase coverage by 0.05%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #238      +/-   ##
==========================================
+ Coverage   96.12%   96.17%   +0.05%     
==========================================
  Files           8        8              
  Lines         748      758      +10     
==========================================
+ Hits          719      729      +10     
  Misses         29       29              
Impacted Files Coverage Δ
src/error_hints.jl 83.33% <100.00%> (ø)
src/traits.jl 98.82% <100.00%> (+0.07%) :arrow_up:

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 7f1d47c...138e229. Read the comment docs.

kimikage commented 3 years ago

One of the major differences from PR #186 is the addition of oneunit for XYZ and LMS. Those components are usually normalized. (They can often take values greater than 1, though.) By analogy with RGB, it is reasonable to assume that (1, 1, 1) is the oneunit in XYZ and LMS spaces.

However, the white point in sRGB is D65, not CIE E, so XYZ(Gray(1)) != XYZ(1, 1, 1). Thus, convert(XYZ, 1) is ambiguous, and if one(c) = 1, then ones(XYZ{T}, ...) is unavailable.

In other words, this specification is less about practicality, and more about ensuring consistency with RGB (within ColorVectorSpace.jl), as some of the XYZ and LMS arithmetic operations are implemented in Colors.jl.

kimikage commented 3 years ago

In addition, @timholy suggested moving these to "operations.jl" as well. (cf. https://github.com/JuliaGraphics/ColorTypes.jl/pull/186#discussion_r411328795)

I haven't moved these because I don't see a clear reason.

kimikage commented 3 years ago

I fixed a bug caused by color_type(AGray) != Gray(cf. issue #152), and added a few tests.

kimikage commented 3 years ago

We have one more major task left to complete, i.e. #235, so I will merge this first. Which file to put the methods in is not the essential issue. It can be modified at any time.

cc: @timholy