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
PR: Add TE226 v2 colour checker reference values. #1113
PR: Add TE226 v2 colour checker reference values. #1113
Conversation
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.
Thanks @Rusching!
I left a few notes, beyond those, would it be possible to:
Update the header of the module:
- :attr:`colour.characterisation.datasets.colour_checkers.chromaticity_coordinates. CCS_TE226_V2`: Reference data from *TE226 V2*.
Add citation in alphabetical order:
- :cite:`ImageEngineering2017` : Image Engineering. (2017). TE226 V2 data sheet. https://www.image-engineering.de/content/products/charts/te226/downloads/TE226_D_data_sheet.pdf
Use :cite:ImageEngineering2017
wherever relevant in the module.
Cheers,
Thomas
"patch19": np.array([0.2646, 0.2542, 0.1631]), | ||
"patch20": np.array([0.7921, 0.7560, 0.5988]), | ||
"patch21": np.array([0.4409, 0.4004, 0.3366]), | ||
"patch22": np.array([0.1546, 0.3395, 0.1016]), | ||
"patch23": np.array([0.3182, 0.3950, 0.5857]), | ||
"patch24": np.array([0.5920, 0.5751, 0.9892]), | ||
"patch25": np.array([0.4287, 0.2583, 0.0444]), | ||
"patch26": np.array([0.4282, 0.5757, 0.4770]), | ||
"patch27": np.array([0.1697, 0.1294, 0.7026]), | ||
"patch28": np.array([0.2143, 0.1564, 0.1908]), | ||
"patch29": np.array([0.1659, 0.3876, 0.3945]), | ||
"patch30": np.array([0.1869, 0.1093, 0.7069]), | ||
"patch31": np.array([0.3316, 0.1596, 0.1714]), | ||
"patch32": np.array([0.8298, 0.8910, 0.5199]), | ||
"patch33": np.array([0.1412, 0.1758, 0.4643]), | ||
"patch34": np.array([0.0153, 0.0668, 0.0694]), | ||
"patch35": np.array([0.6053, 0.5088, 0.1593]), | ||
"patch36": np.array([0.4217, 0.4459, 0.3173]), |
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.
Minor
Fine to keep the space between patch
and the patch number. patch19
--> patch 19
.
[ | ||
0.0190, | ||
0.0202, | ||
0.0202, |
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.
Minor
Extra coma after 0.0202
forces an inconsistent formatting wrap.
tuple(DATA_TE226_V2_CIE_XYZ.keys()), | ||
XYZ_to_xyY( | ||
list(DATA_TE226_V2_CIE_XYZ.values()), | ||
CCS_ILLUMINANTS["CIE 1931 2 Degree Standard Observer"]["ICC D50"], |
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.
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.
Thank you for your detailed review and advice! I would push the changes.
Thanks @Rusching! It is merged! :) |
Happy! ^ ^ |
Ha! Excellent :) |
Summary
Added TE226 V2 color checking values for issue: #901
Preflight
Code Style and Quality
colour
,colour.models
.Documentation