tickets/DM-13834: Add minimum and maximum wavelength to FilterProperty. #342
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ktlim
reviewed
Apr 5, 2018
| @@ -246,7 +246,7 @@ def setUp(self): | |||
| lambdaEff in wavelengths.items() if name == "g"][0] # for tests | |||
|
|
|||
| def defineFilterProperty(self, name, lambdaEff, force=False): | |||
| return afwImage.FilterProperty(name, lambdaEff, force) | |||
| return afwImage.FilterProperty(name, lambdaEff, force=force) | |||
There was a problem hiding this comment.
Would be nice to test the new functionality as well as the old.
jmeyers314
approved these changes
Apr 5, 2018
include/lsst/afw/image/Filter.h
Outdated
| @@ -90,6 +92,14 @@ class FilterProperty { | |||
| * Return the filter's effective wavelength (nm) | |||
| */ | |||
| double getLambdaEff() const { return _lambdaEff; } | |||
| /** | |||
| * Return the filter's minimum wavelength (nm) | |||
There was a problem hiding this comment.
Is it possible to be more precise here?
For example, are these the values beyond which < X% of the total throughput exists? or where the fractional throughput first drops below X% of the peak? After all, if we're being really pedantic, I think we expect some light leaks (at the ~10^-5 of peak throughput level) across pretty much the entire visible range from 350nm to 1100nm (but obviously it isn't useful to set lambdaMin=350 and lambdaMax=1100 for every filter). If you do have a concrete procedure for calculating these, or even if the answer is "I eyeballed it" it would be good to document that.
isullivan
force-pushed
the
tickets/DM-13834
branch
from
ceaad0d
to
73a21f1
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds additional FilterProperties
lambdaMinandlambdaMax, and should work in a backwards-compatible way. But, I am not confident working with C++, so I would appreciate an extra-careful look at my changes to make sure I haven't messed something up.