Skip to content

PR: Enable *DIN99* method selection in colour.difference.delta_E_DIN99 definition.#1393

Merged
KelSolaar merged 3 commits intocolour-science:feature/delta_e_din99_methods_stagingfrom
lassefschmidt:feature/delta_e_din99_methods
Mar 14, 2026
Merged

PR: Enable *DIN99* method selection in colour.difference.delta_E_DIN99 definition.#1393
KelSolaar merged 3 commits intocolour-science:feature/delta_e_din99_methods_stagingfrom
lassefschmidt:feature/delta_e_din99_methods

Conversation

@lassefschmidt
Copy link
Copy Markdown
Contributor

Summary

A very small PR to enable selecting the Lab → DIN99 conversion variant when computing ΔE_DIN99.

Both colour.difference.delta_E and colour.models.Lab_to_DIN99 expose a method parameter, which makes it impossible to forward the conversion method via the existing **kwargs mechanism without a keyword collision.

To resolve this, a dedicated din99_method parameter is added to colour.difference.delta_E_DIN99. The parameter defaults to "DIN99", matching the normative DIN99 definition used by Lab_to_DIN99.

The provided value is validated against DIN99_METHODS, which remains the single source of truth for supported DIN99 variants.

Preflight

Code Style and Quality

  • [N/A] Unit tests have been implemented and passed.
  • 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.
TOTAL                                                                          44613   9996    78%
============================= slowest 5 durations =============================
78.73s call     colour/recovery/tests/test_gaussian.py::TestOptimiseGaussianBasisParameters::test_optimise_gaussian_basis_parameters
21.09s call     colour/recovery/tests/test_jakob2019.py::TestLUT3D_Jakob2019::test_size
16.30s call     colour/notation/tests/test_munsell.py::TestxyY_to_munsell_specification::test_xyY_to_munsell_specification
6.93s call     colour/io/luts/lut.py::colour.io.luts.lut.LUT3D.invert
6.55s call     colour/recovery/jakob2019.py::colour.recovery.jakob2019.LUT3D_Jakob2019.write
=========================== short test summary info ===========================
FAILED colour/io/image.py::colour.io.image.read_image_Imageio
FAILED colour/io/tests/test_image.py::TestWriteImageImageio::test_write_image_Imageio_exr
FAILED colour/io/tests/test_image.py::TestReadImageImageio::test_read_image_Imageio
==== 3 failed, 3697 passed, 17 skipped, 225 warnings in 112.27s (0:01:52) =====
===============================================================================
*                                                                             *
*   Checking codebase with "Pyright"...                                       *
*                                                                             *
===============================================================================
0 errors, 0 warnings, 0 informations

Documentation

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

@KelSolaar
Copy link
Copy Markdown
Member

Thanks @lassefschmidt, it is the first time we introduce this pattern, makes me wonder if we should not adopt the machinery that the convert definition uses, e.g., to target the args of sd_to_XYZ specifically:

convert(
    sd,
    "Spectral Distribution",
    "sRGB",
    sd_to_XYZ={"illuminant": SDS_ILLUMINANTS["FL2"]},
)

@KelSolaar
Copy link
Copy Markdown
Member

KelSolaar commented Jan 30, 2026

So yeah something that we might be able to do:

# Layer 1: filter direct kwargs by target function signature
filtered_kwargs = filter_kwargs(conversion_function, **kwargs)

# Layer 2: merge in function-name-keyed dict (overrides)
filtered_kwargs.update(kwargs.get(conversion_function_name, {}))
delta_E(a, b, method="DIN99", delta_E_DIN99={"method": "DIN99b"})

And delta_E_DIN99 could simply have a method parameter (consistent with the other 100+ functions in the codebase):

def delta_E_DIN99(Lab_1, Lab_2, textiles=False, method="DIN99"):
    ...

@lassefschmidt
Copy link
Copy Markdown
Contributor Author

Thanks @lassefschmidt, it is the first time we introduce this pattern, makes me wonder if we should not adopt the machinery that the convert definition uses, e.g., to target the args of sd_to_XYZ specifically:

convert(
    sd,
    "Spectral Distribution",
    "sRGB",
    sd_to_XYZ={"illuminant": SDS_ILLUMINANTS["FL2"]},
)

I had the same thought when writing this PR, but didn’t want to introduce too big of a change. This way, we would also get rid of all the « extra » parameters depending on the function (e.g. the textiles boolean or the l:c values)

We could even allow users to provide ANY color space array for a and b (and a and b must not even have the same color space), the keyword dict for the conversion function to the desired target space and use the conversion graph. Which would create a super convenient delta E wrapper. But possibly overkill !

@KelSolaar KelSolaar changed the title Enable DIN99 method selection in ΔE_DIN99 PR: Enable *DIN99* method selection in colour.difference.delta_E_DIN99 definition. Jan 30, 2026
@KelSolaar
Copy link
Copy Markdown
Member

I tend to prefer API consistency over special cases (I know that we already have plenty!) so if you are happy with suggested changes, please roll!

@lassefschmidt
Copy link
Copy Markdown
Contributor Author

Happy to propose a new implementation then. Give me a few days max and I should push an update to this PR (probably rebased on most recent develop branch).

One clarification : if we extend to arbitrary color space inputs a and b and pass the conversion args for the .convert function, do we also want to « improve » how we treat the different additional arguments of the delta E methods (such as textiles boolean or l:c) ? Can we introduce breaking changes here (e.g. via the keyword dict using the delta E method name as you proposed) or we keep supporting a direct pass of textiles and l:c in the delta E wrapper ?

@KelSolaar
Copy link
Copy Markdown
Member

how we treat the different additional arguments of the delta E methods (such as textiles boolean or l:c) ?

Can you clarify? textiles for example should be filtered and only passed if relevant to a function right?

@lassefschmidt lassefschmidt force-pushed the feature/delta_e_din99_methods branch from 1b3997d to 97bd0ff Compare February 27, 2026 11:50
@lassefschmidt
Copy link
Copy Markdown
Contributor Author

Done !

To avoid breaking changes, you can still pass c, l, and textiles directly to colour.delta_E. But a more precise (even elegant way) would be to use the new kwargs unpacking, which is 100% identical to our process in colour.convert. This avoids e.g. passing the c argument to some delta E function other than colour.delta_E_CMC and having it silently removed.

That is, this still works

import colour
import numpy as np
a = np.array([48.99183622, -0.10561667, 400.65619925])
b = np.array([50.65907324, -0.11671910, 402.82235718])

# c and l will be passed down to delta_E_CMC
colour.delta_E(
    a,
    b,
    "CMC",
    c=1,
    l=5,
)

# c and l will silently be filtered out (obvious if you check function definition of delta_E_DIN99 but not very transparent)
colour.delta_E(
    a,
    b,
    "DIN99",
    c=1,
    l=5,
)

but we should encourage using

# c and l will be passed down to delta_E_CMC
colour.delta_E(
    a,
    b,
    "CMC",
    delta_E_CMC={"c": 1, "l": 5}
)

# c and l will not be applied as we use DIN99, but it is more obvious due to explicit kwargs structure
colour.delta_E(
    a,
    b,
    "DIN99",
    delta_E_CMC={"c": 1, "l": 5}
)

Maybe we should add a warning in v0.4.8 that l and c and textiles will be deprecated ? and then remove them for release v0.4.9 ?

@KelSolaar KelSolaar changed the base branch from develop to feature/delta_e_din99_methods_staging March 14, 2026 21:02
@KelSolaar KelSolaar merged commit 0f33c5a into colour-science:feature/delta_e_din99_methods_staging Mar 14, 2026
15 of 16 checks passed
@KelSolaar
Copy link
Copy Markdown
Member

@lassefschmidt : I merged the PR in a staging branch, will do some micro-tweaks and merge to develop straight!

but we should encourage using ...

This calls for a larger conversation, because the current pattern with kwargs filtering is well established and used throughout the codebase. If we change it, we want to do this holistically and not just for one definition.

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