Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Removing eltype_ub and renaming eltype_default #257

Open
kimikage opened this issue May 31, 2021 · 3 comments
Open

Removing eltype_ub and renaming eltype_default #257

kimikage opened this issue May 31, 2021 · 3 comments
Milestone
0.13

Comments

@kimikage
Copy link
Collaborator

This is part of the discussion on the promotion rules for constructor argument types.

The parametric color types currently defined in ColorTypes can be divided into two groups.

  1. types where the component type T <: Fractional, e.g. RGB, Gray, ARGB, GrayA
  2. types where the component type T <: AbstractFloat, e.g. HSV, ALab

The difference between these two groups affects the type promotion of constructor arguments.

ColorTypes.jl/src/types.jl

Lines 561 to 570 in 754a397

# Upper bound on element type for each color type
eltype_ub(::Type{P}) where {P<:Colorant } = eltype_ub(eltype_default(P))
eltype_ub(::Type{T}) where {T<:FixedPoint } = Fractional
eltype_ub(::Type{T}) where {T<:AbstractFloat} = AbstractFloat
@inline promote_eltype(::Type{C}, vals...) where {C<:Colorant} = _promote_eltype(eltype_ub(C), eltype_default(C), promote_type(map(typeof, vals)...))
_promote_eltype(::Type{AbstractFloat}, ::Type{Tdef}, ::Type{T}) where {Tdef,T<:AbstractFloat} = T
_promote_eltype(::Type{AbstractFloat}, ::Type{Tdef}, ::Type{T}) where {Tdef,T<:Real} = Tdef
_promote_eltype(::Type{Fractional}, ::Type{Tdef}, ::Type{T}) where {Tdef,T<:Fractional} = T
_promote_eltype(::Type{Fractional}, ::Type{Tdef}, ::Type{T}) where {Tdef,T<:Real} = Tdef

For example:

julia> ColorTypes.promote_eltype(RGB, 0, 0, 0)
N0f8 (alias for Normed{UInt8, 8})

julia> ColorTypes.promote_eltype(HSV, 0, 0, 0)
Float32

julia> ColorTypes.promote_eltype(RGB, 0N0f8, 0N0f8, 0N0f8)
N0f8 (alias for Normed{UInt8, 8})

julia> ColorTypes.promote_eltype(HSV, 0N0f8, 0N0f8, 0N0f8)
Float32

However, I would like to relax the upper bound (cf. #131 (comment)). In that case, eltype_ub for parametric types will always return Real, and that makes no sense. Also, if we follow the current policy of promotion based on the upper bound, we will end up with the following problematic results:

julia> ColorTypes.promote_eltype(RGB, 0, 0, 0) # <:Real
Int64

julia> ColorTypes.promote_eltype(HSV, 0, 0, 0) # <:Real
Int64

julia> ColorTypes.promote_eltype(RGB, 0N0f8, 0N0f8, 0N0f8) # <:Real
N0f8 (alias for Normed{UInt8, 8})

julia> ColorTypes.promote_eltype(HSV, 0N0f8, 0N0f8, 0N0f8) # <:Real
N0f8 (alias for Normed{UInt8, 8})

This means that we need to move away from the policy based on the upper bound. Then the private API eltype_ub is no longer needed.
@kimikage
Copy link
Collaborator Author

The current rules in promote_eltype semantically involve determining whether the component type can be FixedPoint or not.

However, the difference between fixed-point and floating-point has essentially nothing to do with the color models. For example, the 32-bit fixed-point type Q15f16 has sufficient range and depth of resolution for most color types. For example, both RGB{Q15f16} and HSV{Q15f16} are reasonable types. On the other hand, since hue requires a range of [0,360), obviously HSV{N0f8} is not a reasonable type, while RGB{N0f8} is. So, what is important is the range of values that the color model requires.

Therefore, a rule based on gamutmin/gamutmax is more appropriate, but gamutmin/gamutmax is somewhat ambiguous and does not suitable for the type inference.

I believe that eltype_default can be used instead of gamutmin/gamutmax. The range of values that eltype_default can represent implies the range of values that the color model requires.

Interestingly, even in the current implementation, the return value of eltype_ub is actually determined based on the eltype_default.

ColorTypes.jl/src/types.jl

Line 562 in 754a397

eltype_ub(::Type{P}) where {P<:Colorant } = eltype_ub(eltype_default(P))

@kimikage
Copy link
Collaborator Author

Now, before we get into the discussion of specific promotion rules, there is a decision to be made, i.e. the naming of eltype_default.

When a user defines a new color type, the user will be allowed to control the behavior of the constructors by overloading the eltype_default.
Thus, eltype_default has not been exported, nor should it be in the future, but it stands as a quasi-public API.

However, eltype may be deprecated in the future (cf. #184). Since PR #184 was submitted, I've been consciously using "component type" instead of "eltype".
Also, when eltype_ub is removed, the postfix modifier of _default is a bit uncomfortable.

Note that there is no need to remove eltype_default. We can leave it as the alias for compatibility.

@kimikage
Copy link
Collaborator Author

BTW, I tried a totally new promotion rule based on eltype_default in PR #197, but Julia v1.0 failed to infer the types. For this reason, I plan to implement rules which do not deviate significantly from the current rules in PR #197, and submit another PR for drastic changes.

@kimikage kimikage added this to the 0.12 milestone May 31, 2021
This was referenced May 31, 2021
Closed
Merged
@kimikage kimikage mentioned this issue Aug 2, 2021
Open
@ViralBShah ViralBShah mentioned this issue Oct 21, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.13
Development

No branches or pull requests
1 participant
@kimikage