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

PR: Add default shape argument to colour.quality.cfi2017.sd_reference_illuminant definition. #1320

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 11 additions & 7 deletions colour/quality/cfi2017.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@
if typing.TYPE_CHECKING:
from colour.hints import ArrayLike, List, Literal, Tuple

from colour.hints import NDArrayFloat, cast
from colour.colorimetry.spectrum import SPECTRAL_SHAPE_DEFAULT
from colour.hints import ArrayLike, List, NDArrayFloat, Tuple, cast
from colour.models import JMh_CIECAM02_to_CAM02UCS, UCS_to_uv, XYZ_to_UCS
from colour.temperature import CCT_to_xy_CIE_D, uv_to_CCT_Ohno2013
from colour.utilities import (
Expand All @@ -66,16 +67,16 @@
__status__ = "Production"

__all__ = [
"SPECTRAL_SHAPE_CIE2017",
"ROOT_RESOURCES_CIE2017",
"DataColorimetry_TCS_CIE2017",
"SPECTRAL_SHAPE_CIE2017",
"CCT_reference_illuminant",
"ColourRendering_Specification_CIE2017",
"DataColorimetry_TCS_CIE2017",
"colour_fidelity_index_CIE2017",
"delta_E_to_R_f",
"load_TCS_CIE2017",
"CCT_reference_illuminant",
"sd_reference_illuminant",
"tcs_colorimetry_data",
"delta_E_to_R_f",
Comment on lines -69 to -78
Copy link
Member

Choose a reason for hiding this comment

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

Question

Why addition/removal/order change of the objects?

]

SPECTRAL_SHAPE_CIE2017: SpectralShape = SpectralShape(380, 780, 1)
Expand Down Expand Up @@ -346,7 +347,9 @@ def CCT_reference_illuminant(sd: SpectralDistribution) -> NDArrayFloat:
return uv_to_CCT_Ohno2013(UCS_to_uv(XYZ_to_UCS(XYZ)), start=1000, end=25000)


def sd_reference_illuminant(CCT: float, shape: SpectralShape) -> SpectralDistribution:
def sd_reference_illuminant(
CCT: float, shape: SpectralShape = SPECTRAL_SHAPE_DEFAULT
) -> SpectralDistribution:
"""
Compute the reference illuminant for a given correlated colour temperature
:math:`T_{cp}` for use in *CIE 2017 Colour Fidelity Index* (CFI)
Expand All @@ -357,7 +360,8 @@ def sd_reference_illuminant(CCT: float, shape: SpectralShape) -> SpectralDistrib
CCT
Correlated colour temperature :math:`T_{cp}`.
shape
Desired shape of the returned spectral distribution.
Desired shape of the returned spectral distribution. Defaults to
SPECTRAL_SHAPE_DEFAULT

Returns
-------
Expand Down
9 changes: 9 additions & 0 deletions colour/quality/tests/test_cfi2017.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
reshape_sd,
sd_blackbody,
)
from colour.colorimetry.spectrum import SPECTRAL_SHAPE_DEFAULT
from colour.quality.cfi2017 import (
CCT_reference_illuminant,
colour_fidelity_index_CIE2017,
Expand Down Expand Up @@ -924,6 +925,14 @@ class TestSdReferenceIlluminant:
definition unit tests methods.
"""

def test_default_args(self) -> None:
"""Test :func:`color.quality.CIE2017.sd_reference_illuminant` for
default shape argument.
"""

sd = sd_reference_illuminant(5421)
assert sd.shape == SPECTRAL_SHAPE_DEFAULT

Comment on lines +928 to +935
Copy link
Member

Choose a reason for hiding this comment

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

Minor

It is fine to have this test in the test_sd_reference_illuminant method. We are not doing "true" unit tests.

def test_sd_reference_illuminant(self) -> None:
"""
Test :func:`colour.quality.CIE2017.sd_reference_illuminant`
Expand Down
Loading