Skip to content
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

datasets: TCS: Adding TCS15 #1337

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

cmuellner
Copy link

@cmuellner cmuellner commented Mar 6, 2025

Summary

TCS15 was added to the CIE 1995 test color samples to get a better representation of the Japanese skin complexion.

This commit adds this color sample and adjust the CRI test file accordingly.

Note, that the TCS15 dataset is published from 380-780 nm in 5 nm increments. This differs from all other TCS data, which is published from 360-830 nm (also in 5 nm increments).

Source of data is the CIE's open access dataset page: https://www.cie.co.at/datatable/spectral-radiance-factors-test-colour-sample-15-japanese-skin-complexion-5nm-wavelength

Data source reference:
CIE 2024, Spectral radiance factors of test-colour sample #15 of the Japanese skin complexion, 5nm wavelength steps, International Commission on Illumination (CIE), Vienna, AT, DOI: 10.25039/CIE.DS.7chm7z5h

Preflight

Code Style and Quality

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

I observed the following fails on the develop branch (06edf82f054 with no changes on top) on my Fedora 41 (x86-64) machine:

FAILED colour/io/image.py::colour.io.image.read_image
FAILED colour/io/image.py::colour.io.image.read_image_Imageio
FAILED colour/io/tests/test_image.py::TestReadImageImageio::test_read_image_Imageio
FAILED colour/io/tests/test_image.py::TestWriteImageImageio::test_write_image_Imageio
FAILED colour/io/tests/test_image.py::TestWriteImage::test_write_image - Valu...
FAILED colour/io/tests/test_image.py::TestReadImage::test_read_image - ValueE...

This commit seems to trigger the following additional fails, but I cannot find an obvious reason for that:

FAILED colour/colorimetry/tests/test_uniformity.py::TestSpectralUniformity::test_spectral_uniformity
FAILED colour/colorimetry/uniformity.py::colour.colorimetry.uniformity.spectral_uniformity

The pre-commit hook triggers the error RuntimeError: failed to find interpreter for Builtin discover of python_spec='python3.10'.

Documentation

  • New features are documented along with examples if relevant.
  • The documentation is Sphinx and numpydoc compliant.

TCS15 was added to the CIE 1995 test color samples to get a better
representation of the Japanese skin complexion.

This commit adds this color sample and adjust the CRI test file
accordingly.

Note, that the TCS15 dataset is published from 380-780 nm in 5 nm increments.
This differs from all other TCS data, which is published from 360-830 nm
(also in 5 nm increments).

This commit seems to trigger the following regressions, but I cannot
find an obvious reason for that:
* FAILED colour/colorimetry/tests/test_uniformity.py::TestSpectralUniformity::test_spectral_uniformity
* FAILED colour/colorimetry/uniformity.py::colour.colorimetry.uniformity.spectral_uniformity

Source of data is the CIE's open access dataset page:
https://www.cie.co.at/datatable/spectral-radiance-factors-test-colour-sample-15-japanese-skin-complexion-5nm-wavelength

Data source reference:
CIE 2024, Spectral radiance factors of test-colour sample colour-science#15 of the Japanese
skin complexion, 5nm wavelength steps, International Commission on Illumination
(CIE), Vienna, AT, DOI: 10.25039/CIE.DS.7chm7z5h

Signed-off-by: Christoph Müllner <[email protected]>
@cmuellner
Copy link
Author

Force-pushed the following changes:

  • White-space issues identified by pre-commit.ci

@KelSolaar
Copy link
Member

Hi @cmuellner,

Thank you! The image io related tests failure is related to the fact that you probably don't have the Freeimage backend installed: uv run python -c "import imageio;imageio.plugins.freeimage.download()"

The spectral uniformity tests are on the other hand a side effect to the fact you added a new TCS sample which leads me to the following point: We should not modify the existing samples because this will change CRI and everything that depends on it. What we should probably do instead is:

  • Deprecate the colour.quality.datasets.tcs.SDS_TCS attribute and rename it to colour.quality.datasets.tcs.SDS_TCS_CIE_1995
  • Create a new colour.quality.datasets.tcs.SDS_TCS_CIE_2024 attribute with the Japanese skin sample
  • Add a new method argument to the colour.colour_rendering_index definition that takes two values, e.g., ["CIE 1995", "CIE 2024"] allowing to switch to between the two datasets. The colour.colour_quality_scale definition can be used as an example on how to proceed
  • Add unit tests for the new method

Let me know if that makes sense!

Cheers,

Thomas

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