fix(color): improve precision in conversions and add tests#217
fix(color): improve precision in conversions and add tests#217vlakoff wants to merge 7 commits intoBacon:mainfrom
Conversation
Previous code was instead handling rgb(255,255,255) as a specific case, which was useless.
…version These coefficients were cluelessly truncated.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #217 +/- ##
============================================
+ Coverage 70.81% 71.95% +1.14%
- Complexity 994 996 +2
============================================
Files 49 49
Lines 3169 3170 +1
============================================
+ Hits 2244 2281 +37
+ Misses 925 889 -36 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
d7d5a4d to
2e46629
Compare
…cision loss Notably, this fixes gray(100) to rgb(255,255,255) instead of rgb(254,254,254).
2e46629 to
666a162
Compare
|
This looks generally good and I'm fine with AI generated tests, as long as they are user-reviewed to make sure they cover the cases as intended. What I'm unhappy with are all the comments usually inserted by AI models. Code should usually be self-documenting, comments should only be required either in doc blocks to expand on the class/function names or inline if there are edge cases which cannot be expressed by the code itself. Can you go over the diff and remove unnecessary comments? |
|
I’ve reviewed these tests, and they’re solid — definitely better than if I had tediously written them myself by hand. Frankly, I could have skipped mentioning they were AI‑generated and nobody would have noticed. As for the comments, I find them really useful to get an efficient overview of the code. For these test classes, I don’t even bother reading the implementation first, I just scan the comments — much more efficient. That said, if you point out specific comments you think should go, I’ll consider removing them. |
7543c74 to
367ab47
Compare
Similar to input 50, input 90 is the only other case that depends on using "255/100" in order to be rounded up instead of down. For these cases, rounding down would yield a different but equally linear progression, but inconsistent with expected calculation results.
367ab47 to
0f46d47
Compare
This fixes #184.
Also added unit tests to cover rgb ↔ cmyk ↔ gray conversions.
Disclaimer: I hate writing tests, so these are fully AI-generated. Though, they work perfectly, and cover all the changes I've made in this PR.