search

JuliaGraphics / ColorTypes.jl

Basic color definitions and traits
Other
78 stars 36 forks source link

[RFC] Decision on the definition of `one` #235

Closed kimikage closed 3 years ago

kimikage commented 3 years ago

We have been discussing the definition of one in PR #175 and #186 (as well as in some other places). However, I don't think the final decision has been made. I organize the candidates.

1. Define one(::Type{<:Colorant}) = 1

If we take a color as a vector, it is reasonable to assume that the real number 1 is the identity for multiplication. Also, ColorTypes.jl has long announced one(Gray) = 1 in depwarn (Most end users probably don't see depwarns anymore, though.). We have not discussed anything other than grays and rgbs, but this idea can be generalized to Colorant, or at least should be generalized as a fallback. The disadvantage of this idea is that 1 is not the "element" of a set of colors or color vectors. Of course, we have the oneunit. Another problem is that Int is somewhat "strong" in the promotion rules. For now, we are choosing a policy of special treatment for integer arguments in color constructors (cf. #177, #197).

2. Remove one(::Type{<:Gray}) form ColorTypes.jl

As long as the definition of color multiplication is ambiguous and it is not defined in ColorTypes.jl, it is reasonable to not define one. We have the error hinting now, which is better than the "invisible" depwarn. Of course, this can lead to inconsistent definitions of one.

3. Define one(::Type{C}) where {C<:Colorant} = oneunit(C)

This is reasonable if we define color multiplication (*) to be element-wise. Since the * operator is still "free" in ColorVectorSpace, there is room to add the definition. The problem with this proposal is that it is different from the existing policy (at least the depwarn). Of course, neither ColorTypes.jl nor ColorVectorSpace.jl have reached v1 yet, so the policy change itself is not a problem.

cc: @timholy, @johnnychen94

kimikage commented 3 years ago

I don't have a strong preference for these options, but I hope to have a conclusion soon. I think 3. is not too bad. However, in that case, I think it would be better to have the definition in ColorVectorSpace. Therefore, I would like to give priority to 2.

kimikage commented 3 years ago

I think another variant of 1. would be to add one(::Type{C}) where {T, C <: Colorant{T}} = one(T), too. Also, I think one(::Type{<:Colorant}) = true is too tricky.

johnnychen94 commented 3 years ago

Being out of track for quite a long time, I don't have a preference and would just follow whatever decision you made here.