colour-science / colour

Colour Science for Python

https://www.colour-science.org

BSD 3-Clause "New" or "Revised" License

2.14k stars 263 forks source link

PR: Fix incorrect `colour.colorimetry.sd_gaussian_fwhm` definition output. #1184

Closed KelSolaar closed 1 year ago

KelSolaar commented 1 year ago

Summary

The colour.colorimetry.sd_gaussian_fwhm output was not correct as per #1171.

Preflight

Code Style and Quality

[x] Unit tests have been implemented and passed.
[x] Pyright static checking has been run and passed.
[x] 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.
[x] The documentation is Sphinx and numpydoc compliant.

coveralls commented 1 year ago

coverage: 99.775%. remained the same when pulling 2f51cc63b681ca5ee3f7049a8a4ca54a214ea399 on feature/sd_gaussian into 1966033e6112b650f51a6932ea4166dc60e36575 on develop.

tjdcs commented 1 year ago

@KelSolaar This is still wrong:

from colour.colorimetry.generation import sd_gaussian_fwhm
from colour.colorimetry.spectrum import SPECTRAL_SHAPE_DEFAULT
from colour.plotting.colorimetry import plot_single_sd
from matplotlib import pyplot as plt
center = 500
fwhm = 10

spd = sd_gaussian_fwhm(center,fwhm,SPECTRAL_SHAPE_DEFAULT)
fig, ax = plot_single_sd(spd, standalone=False)
ax.set_xlim(450,550)
ax.set_xticks([center-fwhm, center-fwhm/2, center, center+fwhm/2, center+fwhm])
ax.set_yticks([0,.5,1])
ax.grid(visible=True)
plt.show()

The acronym FWHM is shorthand for "full width at half maximum" meaning at center + half the fwhm, the spd should have a value of 0.5

tjdcs commented 1 year ago

Ugh.... sorry. My mistake. My local pull to check this didn't go through.

KelSolaar commented 1 year ago

I added a unit test to verify the width.