[RFC] Redesign color constructors #197
Conversation
Codecov Report
@@ Coverage Diff @@
## master #197 +/- ##
==========================================
+ Coverage 82.81% 83.76% +0.94%
==========================================
Files 8 8
Lines 745 733 -12
==========================================
- Hits 617 614 -3
+ Misses 128 119 -9
Continue to review full report at Codecov.
|
|
Took a glance (didn't review since it's marked Draft), this looks quite promising! |
kimikage
force-pushed
the
redesign_ctor
branch
4 times, most recently
from
5ec3662 to
6533522
Compare
|
I made the ambiguity resolution simpler than when this PR was originally submitted. Now it works without explicitly writing constructors. This means that there is no ambiguity with the default constructors. |
kimikage
force-pushed
the
redesign_ctor
branch
5 times, most recently
from
08d6143 to
e4977cf
Compare
|
Simplifying the macro looks promising to me. Just a simple loading time test on this that might worth noting. (No objection on this PR) julia> versioninfo()
Julia Version 1.6.0-rc1
Commit a58bdd9010 (2021-02-06 15:49 UTC)
Platform Info:
OS: macOS (x86_64-apple-darwin18.7.0)
CPU: Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
WORD_SIZE: 64
LIBM: libopenlibm
LLVM: libLLVM-11.0.1 (ORCJIT, skylake)
julia> @time using ColorTypes
# master: 0.266565 seconds (506.97 k allocations: 37.554 MiB, 5.03% gc time, 18.80% compilation time)
# PR 0.308603 seconds (646.04 k allocations: 50.311 MiB, 4.10% gc time, 13.67% compilation time) |
|
I'm not going to make any performance related tweaks in this PR unless there is a very large regression. |
kimikage
changed the title
This eliminates the macro-based constructor generation. Instead, this uses the abstract types to provide the constructor interfaces. This also allows grays to be used in the constructor argument in more cases.
|
This is a drastic change and I have not been able to fully understand the impact of it. Well, I've been struggling with the precompilation problems in Colors.jl for over a year now. I don't exactly know the impact of this change, but at least I know this won't have a fatal negative impact. So, I plan to merge this in around this weekend. cc: @johnnychen94 |
|
I believe we'll live well with it. |
|
I'll merge this, but it won't be released for a while yet. |
This reverts commit 87364c4.
This eliminates the macro-based constructor generation. This helps add user-defined types (cf. #179).
This also allows grays to be used in the constructor argument in more cases (cf. #177 (comment)).
Fixes #156