Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

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

Merged
merged 1 commit into from
Jul 23, 2023

Conversation

KelSolaar
Copy link
Member

@KelSolaar KelSolaar commented Jul 22, 2023

Summary

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

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.
  • [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.
  • The documentation is Sphinx and numpydoc compliant.

@KelSolaar KelSolaar force-pushed the feature/sd_gaussian branch from a2e89cf to 2f51cc6 Compare July 23, 2023 00:01
@coveralls
Copy link

coveralls commented Jul 23, 2023

Coverage Status

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

@KelSolaar KelSolaar merged commit 708ded0 into develop Jul 23, 2023
@KelSolaar KelSolaar deleted the feature/sd_gaussian branch July 23, 2023 01:05
@tjdcs
Copy link
Contributor

tjdcs commented Jul 23, 2023

@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()
Screen Shot 2023-07-22 at 9 32 41 PM

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
Copy link
Contributor

tjdcs commented Jul 23, 2023

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

Screen Shot 2023-07-22 at 9 37 56 PM

@KelSolaar
Copy link
Member Author

I added a unit test to verify the width.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants