search

JuliaGraphics / ColorTypes.jl

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

[RFC] (Re-)define `one` for `Colorant` #243

Closed kimikage closed 3 years ago

kimikage commented 3 years ago

This defines one as a real (i.e. dimensionless) number １.

I am not convinced that this is the best way, but I think this is a reasonable option.

Since this does not support ones(ARGB, ...) and so on, this may requires PR #177 ~as a part of v0.11.0~

Closes #235, Closes #137

codecov[bot] commented 3 years ago

Codecov Report

Merging #243 (19629f0) into master (213e0de) will increase coverage by 0.10%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #243      +/-   ##
==========================================
+ Coverage   82.62%   82.73%   +0.10%     
==========================================
  Files           8        8              
  Lines         754      753       -1     
==========================================
  Hits          623      623              
+ Misses        131      130       -1
Impacted Files Coverage Δ
src/ColorTypes.jl 100.00% <ø> (ø)
src/error_hints.jl 100.00% <100.00%> (+10.00%) :arrow_up:
src/traits.jl 98.88% <100.00%> (+0.01%) :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 213e0de...19629f0. Read the comment docs.

kimikage commented 3 years ago

I'm not sure I fully understand what @timholy wants to do with ColorVectorSpace v0.9. For now, ColorVectorSpace is allowing breaking changes to remain at v0.9, rather than "skipping" to ColorVectorSpace v0.10. (Incidentally, I'd rather there be no gaps than to strictly adhere to the SemVer principles.)

The options can be roughly divided into two approaches.

One is that ColorTypes v0.11 will not include this PR, and only define zero and oneunit (and <). This is a "formally" safe approach, but "in practice" it may cause confusion about the support for ColorVectorSpace v0.9 in the downstream packages.

The other is to include this PR and PR #177 in ColorTypes v0.11 and break the definition of one in ColorVectorSpace v0.9.x, and drop the support for ColorTypes v0.10. This seems to be a hard landing, but the package manager should avoid the crash.:sweat_smile:

kimikage commented 3 years ago

In the meantime, I will make some trivial PRs for ColorVectorSpace.

kimikage commented 3 years ago

ImageCore v0.9.0, which depends on ColorVectorSpace v0.9, has been released. ~Currently it should not be available yet~, but some packages will start supporting ColorVectorSpace v0.9. Therefore, releasing ColorVectorSpace v0.10 may be the right way to go.

kimikage commented 3 years ago

I've updated the plan: #231. I'll merge PR ~#243~ #246 and another migration PR (Edit: #248), and release v0.11.0 by the beginning of next week. v0.12.0 is a bit heavier, but I hope to release it by the end of May. I think FixedPointNumbers v0.9 will be the next target. Of course, this is a personal observation and not a definitive one, for better or worse.

kimikage commented 3 years ago

Ah, the error hints for color arithmetic may be v0.11 material. I'm going to split the PR further.

johnnychen94 commented 3 years ago

Since error hints don't change the behavior so it's okay to ship it in v0.12 just to save some effort; I'll just treat this laziness as "whoever wants the new fancy feature, use the new version."

kimikage commented 3 years ago

I have submitted a separated PR #249. I am going to remove duplicate changes when I apply rebase to this PR. That is, I will leave this PR as is for a while.