-
Notifications
You must be signed in to change notification settings - Fork 34
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.
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
Fix show for Ufixed #9
Conversation
I'm thinking this repo should get moved to JuliaLang at this point and relieve @JeffBezanson of the sole responsibility for it. |
Related discussion in #3. I'm fine with whatever, I just thought Jeff might like to have a package 😄. But I agree he's a busy guy. |
|
||
show{T<:Ufixed}(io::IO, ::Type{T}) = print(io, "Ufixed", nbitsfrac(T)) | ||
show{T,f}(io::IO, ::Type{UfixedBase{T,f}}) = print(io, "Ufixed", f) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see there was a definition like this already, but I'm generally very against redefining how types or other low-level things are printed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, we can get rid of it.
The main motivation was that, without it, looking at a 5x5 block of an image gives you this:
5x5 Array{RGB{UfixedBase{Uint8,8}},2}:
RGB{UfixedBase{Uint8,8}}(Ufixed8(0.467),Ufixed8(0.212),Ufixed8(0.871)) RGB{UfixedBase{Uint8,8}}(Ufixed8(0.353),Ufixed8(0.6),Ufixed8(0.804)) … RGB{UfixedBase{Uint8,8}}(Ufixed8(0.431),Ufixed8(0.796),Ufixed8(0.522))
RGB{UfixedBase{Uint8,8}}(Ufixed8(0.075),Ufixed8(0.392),Ufixed8(0.984)) RGB{UfixedBase{Uint8,8}}(Ufixed8(0.231),Ufixed8(0.31),Ufixed8(0.412)) RGB{UfixedBase{Uint8,8}}(Ufixed8(0.337),Ufixed8(0.349),Ufixed8(0.361))
RGB{UfixedBase{Uint8,8}}(Ufixed8(0.851),Ufixed8(0.62),Ufixed8(0.043)) RGB{UfixedBase{Uint8,8}}(Ufixed8(0.62),Ufixed8(0.333),Ufixed8(0.6)) RGB{UfixedBase{Uint8,8}}(Ufixed8(0.102),Ufixed8(0.737),Ufixed8(0.212))
RGB{UfixedBase{Uint8,8}}(Ufixed8(0.671),Ufixed8(0.91),Ufixed8(0.529)) RGB{UfixedBase{Uint8,8}}(Ufixed8(0.596),Ufixed8(0.251),Ufixed8(0.918)) RGB{UfixedBase{Uint8,8}}(Ufixed8(0.125),Ufixed8(0.424),Ufixed8(0.067))
RGB{UfixedBase{Uint8,8}}(Ufixed8(0.365),Ufixed8(0.867),Ufixed8(0.729)) RGB{UfixedBase{Uint8,8}}(Ufixed8(0.22),Ufixed8(0.49),Ufixed8(0.616)) RGB{UfixedBase{Uint8,8}}(Ufixed8(0.584),Ufixed8(0.8),Ufixed8(0.686))
which is quite a mouthful. But I suppose the better approach is to change this in terms of how ColorValues (not types) are represented?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, better to handle it through ColorValue printing.
This fixes a bug that cropped up in the display of methods written in terms of
T<:Ufixed
. Since you can't callnbitsfrac
on aUfixed
(it needs to be aUfixedBase
), it triggered an error.