gain_corrected in ccd_process #491
Conversation
|
@crawfordsm It seems you're not linked to your commits. I had a similar issue lately and even though I don't know what exactly fixed it I bookmarked this github help page on "Commits are not linked to any user" - so I guess I found the answer there. Or is that intended? |
|
Yes, apparently I've had my email set wrong on this new machine due to a typo -- I'm amazed by how many commits I've been allowed to make with that set wrong |
|
@crawfordsm - You can add that email address to your github account, and the it will be recognized. Alternatively listing it in the |
ccdproc/tests/test_ccdproc.py
Outdated
| @@ -915,3 +915,39 @@ def test_ccd_process(): | |||
| assert(occd.unit == u.electron) | |||
| # Make sure the original keyword is still present. Regression test for #401 | |||
| assert occd.meta['testkw'] == 100 | |||
|
|
|||
| def test_ccd_process_gain_corrected(): | |||
| # test the through ccd_process | |||
There was a problem hiding this comment.
This sentence is missing something :)
CHANGES.rst
Outdated
| @@ -13,6 +13,10 @@ New Features | |||
| - Add ``bitfield_to_boolean_mask`` function to convert a ``bitfield`` to a | |||
| boolean mask (following the numpy conventions). [#460] | |||
|
|
|||
| - Added ``gain_corrected`` option in ccd_process so that | |||
There was a problem hiding this comment.
Were the line breaks at 60 intentional?
ccdproc/core.py
Outdated
| @@ -262,6 +265,10 @@ def ccd_process(ccd, oscan=None, trim=None, error=False, master_bias=None, | |||
| raise TypeError( | |||
| 'master_flat is not None or a CCDData object.') | |||
|
|
|||
| # apply the gain correction only at the end if gain_corrected is False | |||
| if isinstance(gain, Quantity) and gain_corrected is False: | |||
There was a problem hiding this comment.
not gain_corrected seems better than gain_corrected is False and it would prevent accidentally not triggering the branch.
| elif gain is None: | ||
| pass | ||
| else: | ||
| if not (gain is None or isinstance(gain, Quantity)): |
There was a problem hiding this comment.
the isinstance(gain, Quantity) is used 3 times in the function now. You could use an intermediate variable so it's only computed once.
There was a problem hiding this comment.
yes, can be removed later on as it is already checked and simplified the code
| @@ -165,6 +165,10 @@ def ccd_process(ccd, oscan=None, trim=None, error=False, master_bias=None, | |||
| If True, scale the dark frame by the exposure times. | |||
| Default is ``False``. | |||
|
|
|||
| gain_corrected : bool, optional | |||
There was a problem hiding this comment.
I'm not exactly sure that's the best approach because the name could also mean that the "object" has been gain corrected.
There was a problem hiding this comment.
It is explained in the string describing what it is referring to
There was a problem hiding this comment.
what about calling the parameter gain_corrected_masters?
ccdproc/core.py
Outdated
| @@ -165,6 +165,10 @@ def ccd_process(ccd, oscan=None, trim=None, error=False, master_bias=None, | |||
| If True, scale the dark frame by the exposure times. | |||
| Default is ``False``. | |||
|
|
|||
| gain_corrected : bool, optional | |||
| If True, the calibration frames have already been gain corrected. | |||
There was a problem hiding this comment.
Maybe a hint which parameters correspond to the "calibration frames" could be added.
This Fixes #490 and adds the option to include
gain_corrected. If set to False, the calibrations files will be assumed to be un-gained corrected and the gain correction will only occur after those corrections are applied to the gain correctionFor new functionality:
Please note that the last point is not a requirement. It is meant as a check if
the pull request potentially breaks backwards-compatibility.
Default value for
gain_correctedis True, which is inline with backward compatibility