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
DM-42160: eo_pipe/cp_verify parity: dark #294
Conversation
dca39b0
to
e84f09a
Compare
python/lsst/ip/isr/isrStatistics.py
Outdated
@@ -104,6 +104,12 @@ class IsrStatisticsTaskConfig(pexConfig.Config): | |||
} | |||
) | |||
|
|||
doCalibDistributionStatistics = pexConfig.Field( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
doCalibDistributionStatistics = pexConfig.Field( | |
doCopyCalibDistributionStatistics = pexConfig.Field( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The field appears to be about copying the distribution statistics, not whether it should compute them or not.
python/lsst/ip/isr/isrStatistics.py
Outdated
ampStats = {} | ||
|
||
for calibType in ('bias', 'dark', 'flat'): | ||
if calibType in kwargs and kwargs[calibType] is not None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if calibType in kwargs and kwargs[calibType] is not None: | |
if kwargs.get(calibType, None) is not None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would either of these work if no keyword arguments are passed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Taking the more compact suggestion. Even if nothing is passed, kwargs still is defined in this scope, so it should be fine.
python/lsst/ip/isr/isrStatistics.py
Outdated
for amp in inputExp.getDetector(): | ||
ampStats = {} | ||
|
||
for calibType in ('bias', 'dark', 'flat'): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for calibType in ('bias', 'dark', 'flat'): | |
for calibType in ("bias", "dark", "flat"): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this in the dev guide, or is this convention to align with packages that are running black? I've accepted the suggestion (I know I'm horribly inconsistent with string quoting), but wanted to know where to point others on this topic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
black always uses double quotes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The dev guide only says to be consistent on quoting within a single file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, given that it's single quotes elsewhere, it can stay as they are now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've replaced all single with double quotes in this file, and will try to be more consistent in the future.
d0e380a
to
35ab779
Compare
Add method to copy calibration statistics to isrStatistics.