Skip to content

PR: Add TE226 v2 colour checker reference values.#1113

Merged
KelSolaar merged 3 commits intocolour-science:developfrom
Rusching:feature/add_te226_reference_values
Mar 4, 2023
Merged

PR: Add TE226 v2 colour checker reference values.#1113
KelSolaar merged 3 commits intocolour-science:developfrom
Rusching:feature/add_te226_reference_values

Conversation

@Rusching
Copy link
Copy Markdown
Contributor

@Rusching Rusching commented Mar 2, 2023

Summary

Added TE226 V2 color checking values for issue: #901

Preflight

Code Style and Quality

  • [N/A] Unit tests have been implemented and passed.
  • [N/A] Pyright static checking has been run and passed.
  • Pre-commit hooks have been run and passed.
  • [N/A] New transformations have been added to the Automatic Colour Conversion Graph.
  • [N/A] New transformations have been exported to the relevant namespaces, e.g. colour, colour.models.

Documentation

  • [N/A] New features are documented along with examples if relevant.
  • [N/A] The documentation is Sphinx and numpydoc compliant.

Copy link
Copy Markdown
Member

@KelSolaar KelSolaar left a 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

Comment on lines +409 to +426
"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]),
Copy link
Copy Markdown
Member

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,
Copy link
Copy Markdown
Member

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"],
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Major

The whitepoint is actually D65:

image

image

Copy link
Copy Markdown
Contributor Author

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.

@KelSolaar KelSolaar changed the title add te226 v2 reference values PR: Add TE226 v2 colour checker reference values. Mar 4, 2023
@KelSolaar KelSolaar merged commit edc8b55 into colour-science:develop Mar 4, 2023
@KelSolaar
Copy link
Copy Markdown
Member

Thanks @Rusching! It is merged! :)

@KelSolaar KelSolaar added this to the v0.4.3 milestone Mar 4, 2023
@Rusching
Copy link
Copy Markdown
Contributor Author

Rusching commented Mar 5, 2023

Happy! ^ ^

@KelSolaar
Copy link
Copy Markdown
Member

Ha! Excellent :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants