-
Notifications
You must be signed in to change notification settings - Fork 0
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
Rename package and redesign types #5
Conversation
Codecov Report
@@ Coverage Diff @@
## main #5 +/- ##
==========================================
- Coverage 67.07% 60.00% -7.08%
==========================================
Files 6 6
Lines 82 95 +13
==========================================
+ Hits 55 57 +2
- Misses 27 38 +11
Continue to review full report at Codecov.
|
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.
The conceptual changes look good to me. Although I'm not sure if AbstractColorChannels
and ColorChannels
(plural form) are good names for types -- we usually use plural forms for modules. How about AbstractMultiChannelColor
and MultiChannelColor
?
We might need to support alias for specific channels -- I'm thinking of something like this:
const MColor2Channels = (RGB(0, 0.5, 0.5), RGB(1, 0, 0))
const MColor2{T} = ColorMixture{T,2,MColor2Channels}
MColor2{Float32}(0.8, 0.2) # equivalent to `ColorMixture(MColor2Channels, (0.8, 0.2))`
For ColorMixture
, to avoid confusions. we need to clearly document that it is not RGB
colorspace:
length(T)
is not3
mapc
applies to the original channels and not to the RGB channels- etc
This creates an abstract `AbstractColorChannels` type and two types, `ColorChannels` and `ColorMixture`. `ColorChannels` is a "bare" multichannel color that lacks conversion to other color types; it is intended to be used in conjunction with MappedArrays if one wishes to visualize these as RGB images. `ColorMixture` is the weighted-RGB color type. The package name has been changed to reflect the wider applicability. This also adds Documenter documentation and slims the README.
Addressed the points above. Now with greatly expanded documentation. The docs talk about arithmetic operations, but those will be implemented in the next PR. |
The color types in this package support two fundamental categories of operations: | ||
|
||
- arithmetic operations such as `+` and `-` and multiplying or dividing by a scalar. You can also scale each color channel independently with `⊙` (obtained with `\odotTAB`) or its synonym `hadamard`, e.g., `g ⊙ c` where `c` is a color object defined in this package and `g` is a tuple of real numbers (the "gains"). | ||
- extracting the independent channel intensities as a tuple with `Tuple(c)`. |
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.
TIL 😆
|
||
Use the package interactively or in code with | ||
|
||
```jldoctest demo |
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.
Just a confirmation: is the jldoctest
deliberately used here rather than the @repl
block?
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, though there are many ways to solve the same problem. Basically I want doctests to make sure the code keeps working, and I don't want to have to use setup=:(using MultiChannelColors)
on each doctest. So I created a named doctest, demo
, and am loading the package in the first instance.
julia> typeof(ctemplate) | ||
ColorMixture{Float32, 2, (RGB{N0f8}(0.0,1.0,0.0), RGB{N0f8}(1.0,0.0,0.0))} | ||
|
||
julia> c = ctemplate(0.2, 0.4) |
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.
neat!
```julia | ||
A = rand(0x0000:0x0fff, 100, 100, 11); | ||
img = StructArray{MultiChannelColor{N4f12,11}}(A; dims=3) | ||
``` |
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.
If we provide a better tool in ImageCore in the spirit of JuliaImages/ImageCore.jl#179, then this story might be changed.
Might be worth leaving a note about future improvement.
I was thinking of something like:
module ImageLayouts
const CHW = DenseImageLayout{:CHW}()
const CWH = DenseImageLayout{:CWH}()
const HWC = DenseImageLayout{:HWC}()
end
so that users can use a unified interface:
using ImageCore # which exports the ImageLayouts module
colorview(ImageLayouts.CHW, img)
we might end up with a new function, though -- pretty much like the strel
concept I introduced in ImageMorphology.
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, I really like the idea of these custom layouts!
If you'll be hard-coding the name of the fluorophore, consider using a slightly different syntax: | ||
|
||
```jldoctest demo | ||
julia> channels = (fluorophore_rgb"EGFP", fluorophore_rgb"tdTomato") |
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.
Will fluorophore"EGFP"
be easier to use? I'm not sure. -- just that I have never seen underscores used in string macros.
Is there non-RGB fluorophore?
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.
No non-RGB fluorophore currently. I guess I would think that fluorophore"EGFP"
might return an entire object that is a complete characterization of the fluorophore, e.g., full spectra, cross section, quantum yield, and so on (this list only scratches the surface). In contrast, fluorophore_rgb
makes it clear that what you're getting is an RGB value that represents the fluorophore.
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 believe this is my last round review. It looks great to me so feel free to merge when you think ready.
Co-authored-by: Johnny Chen <[email protected]>
This creates an abstract
AbstractColorChannels
type and two types,ColorChannels
andColorMixture
.ColorChannels
is a "bare"multichannel color that lacks conversion to other color types;
it is intended to be used in conjunction with MappedArrays if one
wishes to visualize these as RGB images.
ColorMixture
is theweighted-RGB color type.
Closes #2