From 7b03b503916d84b3a7cd3de911eb2b67e28edf26 Mon Sep 17 00:00:00 2001 From: Matthew Craig Date: Mon, 9 Dec 2019 21:42:40 -0700 Subject: [PATCH 01/15] Add regression test for gain correction in lacosmic --- ccdproc/tests/test_cosmicray.py | 36 +++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/ccdproc/tests/test_cosmicray.py b/ccdproc/tests/test_cosmicray.py index 20d05a97..52e3df20 100644 --- a/ccdproc/tests/test_cosmicray.py +++ b/ccdproc/tests/test_cosmicray.py @@ -64,6 +64,42 @@ def test_cosmicray_lacosmic_check_data(): cosmicray_lacosmic(10, noise) +@pytest.mark.parametrize('array_input', [True, False]) +@pytest.mark.parametrize('gain_correct_data', [True, False]) +def test_cosmicray_gain_correct(array_input, gain_correct_data): + # Add regression check for #705 and for the new gain_correct + # argument. + # The issue is that cosmicray_lacosmic gain-corrects the + # data and returns that gain corrected data. That is not the + # intent... + ccd_data = ccd_data_func(data_scale=DATA_SCALE) + threshold = 5 + add_cosmicrays(ccd_data, DATA_SCALE, threshold, ncrays=NCRAYS) + noise = DATA_SCALE * np.ones_like(ccd_data.data) + ccd_data.uncertainty = noise + # This may need units at some point. + gain = 2.0 + if array_input: + new_data, cr_mask = cosmicray_lacosmic(ccd_data.data, + gain=gain, + gain_apply=gain_correct_data) + else: + new_ccd = cosmicray_lacosmic(ccd_data, + gain=gain, + gain_apply=gain_correct_data) + new_data = new_ccd.data + cr_mask = new_ccd.mask + # Fill masked locations with 0 since there is no simple relationship + # between the original value and the corrected value. + orig_data = np.ma.array(ccd_data.data, mask=cr_mask).filled(0) + new_data = np.ma.array(new_data, mask=cr_mask).filled(0) + if gain_correct_data: + gain_for_test = gain + else: + gain_for_test = 1.0 + np.testing.assert_allclose(gain_for_test * orig_data, new_data) + + def test_cosmicray_median_check_data(): with pytest.raises(TypeError): ndata, crarr = cosmicray_median(10, thresh=5, mbox=11, From 8dbbd0c2317a40b22c80c7bb37d5044e24b87a0e Mon Sep 17 00:00:00 2001 From: Matthew Craig Date: Thu, 12 Dec 2019 15:18:27 -0700 Subject: [PATCH 02/15] Add gain_apply argument and implementation --- ccdproc/core.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/ccdproc/core.py b/ccdproc/core.py index 75153e88..cb4d3515 100644 --- a/ccdproc/core.py +++ b/ccdproc/core.py @@ -1306,7 +1306,8 @@ def cosmicray_lacosmic(ccd, sigclip=4.5, sigfrac=0.3, satlevel=65535.0, pssl=0.0, niter=4, sepmed=True, cleantype='meanmask', fsmode='median', psfmodel='gauss', psffwhm=2.5, psfsize=7, - psfk=None, psfbeta=4.765, verbose=False): + psfk=None, psfbeta=4.765, verbose=False, + gain_apply=True): r""" Identify cosmic rays through the L.A. Cosmic technique. The L.A. Cosmic technique identifies cosmic rays by identifying pixels based on a variation @@ -1342,6 +1343,10 @@ def cosmicray_lacosmic(ccd, sigclip=4.5, sigfrac=0.3, Gain of the image (electrons / ADU). We always need to work in electrons for cosmic ray detection. Default: 1.0 + gain_apply : bool, optional + If ``True``, return gain-corrected data, otherwise do not gain-correct + the data. Default is ``True`` to preserve backwards compatibility. + readnoise : float, optional Read noise of the image (electrons). Used to generate the noise model of the image. Default: 6.5. @@ -1472,6 +1477,8 @@ def cosmicray_lacosmic(ccd, sigclip=4.5, sigfrac=0.3, psfsize=psfsize, psfk=psfk, psfbeta=psfbeta, verbose=verbose) + if not gain_apply and gain != 1.0: + cleanarr = cleanarr / gain return cleanarr, crmask elif isinstance(ccd, CCDData): @@ -1486,6 +1493,9 @@ def cosmicray_lacosmic(ccd, sigclip=4.5, sigfrac=0.3, # create the new ccd data object nccd = ccd.copy() + if not gain_apply and gain != 1.0: + cleanarr = cleanarr / gain + nccd.data = cleanarr if nccd.mask is None: nccd.mask = crmask From 13b5ba2a9a42471ae31ef5adeb76bb0bc3d89c77 Mon Sep 17 00:00:00 2001 From: Matthew Craig Date: Thu, 12 Dec 2019 16:06:14 -0700 Subject: [PATCH 03/15] Add test for gain with units --- ccdproc/tests/test_cosmicray.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/ccdproc/tests/test_cosmicray.py b/ccdproc/tests/test_cosmicray.py index 52e3df20..83e6a35d 100644 --- a/ccdproc/tests/test_cosmicray.py +++ b/ccdproc/tests/test_cosmicray.py @@ -6,7 +6,7 @@ import pytest from astropy.utils import NumpyRNGContext from astropy.nddata import StdDevUncertainty - +from astropy import units as u from ..core import (cosmicray_lacosmic, cosmicray_median, background_deviation_box, background_deviation_filter) @@ -100,6 +100,19 @@ def test_cosmicray_gain_correct(array_input, gain_correct_data): np.testing.assert_allclose(gain_for_test * orig_data, new_data) +def test_cormicray_lacosmic_accepts_quantity(): + ccd_data = ccd_data_func(data_scale=DATA_SCALE) + threshold = 5 + add_cosmicrays(ccd_data, DATA_SCALE, threshold, ncrays=NCRAYS) + noise = DATA_SCALE * np.ones_like(ccd_data.data) + ccd_data.uncertainty = noise + # This may need units at some point. + gain = 2.0 * u.electron / u.adu + new_ccd = cosmicray_lacosmic(ccd_data, + gain=gain, + gain_apply=True) + + def test_cosmicray_median_check_data(): with pytest.raises(TypeError): ndata, crarr = cosmicray_median(10, thresh=5, mbox=11, From f8a5606d153a588351406fe364970a26562f5ab8 Mon Sep 17 00:00:00 2001 From: Matthew Craig Date: Thu, 12 Dec 2019 16:48:46 -0700 Subject: [PATCH 04/15] Allow for gain with units --- ccdproc/core.py | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/ccdproc/core.py b/ccdproc/core.py index cb4d3515..533b6411 100644 --- a/ccdproc/core.py +++ b/ccdproc/core.py @@ -1339,13 +1339,14 @@ def cosmicray_lacosmic(ccd, sigclip=4.5, sigfrac=0.3, electrons for cosmic ray detection, so we need to know the sky level that has been subtracted so we can add it back in. Default: 0.0. - gain : float, optional + gain : float or `~astropy.units.Quantity`, optional Gain of the image (electrons / ADU). We always need to work in electrons for cosmic ray detection. Default: 1.0 gain_apply : bool, optional - If ``True``, return gain-corrected data, otherwise do not gain-correct - the data. Default is ``True`` to preserve backwards compatibility. + If ``True``, return gain-corrected data, with correct units, otherwise + do not gain-correct the data. Default is ``True`` to preserve + backwards compatibility. readnoise : float, optional Read noise of the image (electrons). Used to generate the noise model @@ -1465,13 +1466,18 @@ def cosmicray_lacosmic(ccd, sigclip=4.5, sigfrac=0.3, updated with the detected cosmic rays. """ from astroscrappy import detect_cosmics + + # If we didn't get a quantity, turn the gain into one that is + # dimensionless. + if not isinstance(gain, u.Quantity): + gain = gain * u.one if isinstance(ccd, np.ndarray): data = ccd crmask, cleanarr = detect_cosmics( data, inmask=None, sigclip=sigclip, - sigfrac=sigfrac, objlim=objlim, gain=gain, readnoise=readnoise, satlevel=satlevel, pssl=pssl, + sigfrac=sigfrac, objlim=objlim, gain=gain.value, niter=niter, sepmed=sepmed, cleantype=cleantype, fsmode=fsmode, psfmodel=psfmodel, psffwhm=psffwhm, psfsize=psfsize, psfk=psfk, psfbeta=psfbeta, @@ -1485,16 +1491,21 @@ def cosmicray_lacosmic(ccd, sigclip=4.5, sigfrac=0.3, crmask, cleanarr = detect_cosmics( ccd.data, inmask=ccd.mask, - sigclip=sigclip, sigfrac=sigfrac, objlim=objlim, gain=gain, readnoise=readnoise, satlevel=satlevel, pssl=pssl, + sigclip=sigclip, sigfrac=sigfrac, objlim=objlim, gain=gain.value, niter=niter, sepmed=sepmed, cleantype=cleantype, fsmode=fsmode, psfmodel=psfmodel, psffwhm=psffwhm, psfsize=psfsize, psfk=psfk, psfbeta=psfbeta, verbose=verbose) # create the new ccd data object nccd = ccd.copy() + + # Remove the gain scaling if it wasn't desired if not gain_apply and gain != 1.0: - cleanarr = cleanarr / gain + cleanarr = cleanarr / gain.value + + # Fix the units if the gain is being applied. + nccd.unit = ccd.unit * gain.unit nccd.data = cleanarr if nccd.mask is None: From 7d817785ed2a0913e57b16f082e784a106151aab Mon Sep 17 00:00:00 2001 From: Matthew Craig Date: Thu, 12 Dec 2019 16:55:28 -0700 Subject: [PATCH 05/15] Add test for readnoise with units --- ccdproc/tests/test_cosmicray.py | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/ccdproc/tests/test_cosmicray.py b/ccdproc/tests/test_cosmicray.py index 83e6a35d..e487c506 100644 --- a/ccdproc/tests/test_cosmicray.py +++ b/ccdproc/tests/test_cosmicray.py @@ -100,19 +100,34 @@ def test_cosmicray_gain_correct(array_input, gain_correct_data): np.testing.assert_allclose(gain_for_test * orig_data, new_data) -def test_cormicray_lacosmic_accepts_quantity(): +def test_cosmicray_lacosmic_accepts_quantity(): ccd_data = ccd_data_func(data_scale=DATA_SCALE) threshold = 5 add_cosmicrays(ccd_data, DATA_SCALE, threshold, ncrays=NCRAYS) noise = DATA_SCALE * np.ones_like(ccd_data.data) ccd_data.uncertainty = noise - # This may need units at some point. + # The units below are the point of the test gain = 2.0 * u.electron / u.adu new_ccd = cosmicray_lacosmic(ccd_data, gain=gain, gain_apply=True) +def test_cosmicray_lacosmic_accepts_quantity_readnoise(): + ccd_data = ccd_data_func(data_scale=DATA_SCALE) + threshold = 5 + add_cosmicrays(ccd_data, DATA_SCALE, threshold, ncrays=NCRAYS) + noise = DATA_SCALE * np.ones_like(ccd_data.data) + ccd_data.uncertainty = noise + gain = 2.0 + # The units below are the point of this test + readnoise = 6.5 * u.electron + new_ccd = cosmicray_lacosmic(ccd_data, + gain=gain, + gain_apply=True, + readnoise=readnoise) + + def test_cosmicray_median_check_data(): with pytest.raises(TypeError): ndata, crarr = cosmicray_median(10, thresh=5, mbox=11, From e59c32f6ad517fe066f3434a34e1001aa00fc288 Mon Sep 17 00:00:00 2001 From: Matthew Craig Date: Thu, 12 Dec 2019 16:58:49 -0700 Subject: [PATCH 06/15] Allow readnoise to have units --- ccdproc/core.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/ccdproc/core.py b/ccdproc/core.py index 533b6411..2e24e4a0 100644 --- a/ccdproc/core.py +++ b/ccdproc/core.py @@ -1471,13 +1471,17 @@ def cosmicray_lacosmic(ccd, sigclip=4.5, sigfrac=0.3, # dimensionless. if not isinstance(gain, u.Quantity): gain = gain * u.one + + if not isinstance(readnoise, u.Quantity): + readnoise = readnoise * u.one + if isinstance(ccd, np.ndarray): data = ccd crmask, cleanarr = detect_cosmics( data, inmask=None, sigclip=sigclip, - readnoise=readnoise, satlevel=satlevel, pssl=pssl, sigfrac=sigfrac, objlim=objlim, gain=gain.value, + readnoise=readnoise.value, satlevel=satlevel, pssl=pssl, niter=niter, sepmed=sepmed, cleantype=cleantype, fsmode=fsmode, psfmodel=psfmodel, psffwhm=psffwhm, psfsize=psfsize, psfk=psfk, psfbeta=psfbeta, @@ -1491,8 +1495,8 @@ def cosmicray_lacosmic(ccd, sigclip=4.5, sigfrac=0.3, crmask, cleanarr = detect_cosmics( ccd.data, inmask=ccd.mask, - readnoise=readnoise, satlevel=satlevel, pssl=pssl, sigclip=sigclip, sigfrac=sigfrac, objlim=objlim, gain=gain.value, + readnoise=readnoise.value, satlevel=satlevel, pssl=pssl, niter=niter, sepmed=sepmed, cleantype=cleantype, fsmode=fsmode, psfmodel=psfmodel, psffwhm=psffwhm, psfsize=psfsize, psfk=psfk, psfbeta=psfbeta, verbose=verbose) From a4cab671726a34b1e4858bc0d084d6922fcc7133 Mon Sep 17 00:00:00 2001 From: Matthew Craig Date: Fri, 13 Dec 2019 10:05:46 -0700 Subject: [PATCH 07/15] Add test for unit consistency --- ccdproc/tests/test_cosmicray.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/ccdproc/tests/test_cosmicray.py b/ccdproc/tests/test_cosmicray.py index e487c506..cf4eb4fd 100644 --- a/ccdproc/tests/test_cosmicray.py +++ b/ccdproc/tests/test_cosmicray.py @@ -128,6 +128,28 @@ def test_cosmicray_lacosmic_accepts_quantity_readnoise(): readnoise=readnoise) +def test_cosmicray_lacosmic_detects_inconsistent_units(): + # This is intended to detect cases like a ccd with units + # of adu, a readnoise in electrons and a gain in adu / electron. + # That is not internally inconsistent. + ccd_data = ccd_data_func(data_scale=DATA_SCALE) + ccd_data.unit='adu' + threshold = 5 + add_cosmicrays(ccd_data, DATA_SCALE, threshold, ncrays=NCRAYS) + noise = DATA_SCALE * np.ones_like(ccd_data.data) + ccd_data.uncertainty = noise + readnoise = 6.5 * u.electron + + # The units below are deliberately incorrect. + gain = 2.0 * u.adu / u.electron + with pytest.raises(ValueError) as e: + cosmicray_lacosmic(ccd_data, + gain=gain, + gain_apply=True, + readnoise=readnoise) + assert 'Inconsistent units' in str(e.value) + + def test_cosmicray_median_check_data(): with pytest.raises(TypeError): ndata, crarr = cosmicray_median(10, thresh=5, mbox=11, From c3bfabcaeaf7e3d517cfa23730e62a89c63f46ab Mon Sep 17 00:00:00 2001 From: Matthew Craig Date: Fri, 13 Dec 2019 10:07:57 -0700 Subject: [PATCH 08/15] Add check for unit consistency and fix tests --- ccdproc/core.py | 17 ++++++++++++----- ccdproc/tests/test_cosmicray.py | 15 +++++++++++---- 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/ccdproc/core.py b/ccdproc/core.py index 2e24e4a0..ff948b70 100644 --- a/ccdproc/core.py +++ b/ccdproc/core.py @@ -1467,13 +1467,15 @@ def cosmicray_lacosmic(ccd, sigclip=4.5, sigfrac=0.3, """ from astroscrappy import detect_cosmics - # If we didn't get a quantity, turn the gain into one that is - # dimensionless. + # If we didn't get a quantity, put them in, with unit specified by the + # documentation above. if not isinstance(gain, u.Quantity): - gain = gain * u.one + gain = gain * u.electron / u.adu + # Set the units of readnoise to electrons, as specified in the + # documentation, if no unit is present. if not isinstance(readnoise, u.Quantity): - readnoise = readnoise * u.one + readnoise = readnoise * u.electron if isinstance(ccd, np.ndarray): data = ccd @@ -1492,7 +1494,12 @@ def cosmicray_lacosmic(ccd, sigclip=4.5, sigfrac=0.3, return cleanarr, crmask elif isinstance(ccd, CCDData): - + # Check unit consistency before taking the time to check for + # cosmic rays. + if not (gain * ccd).unit.is_equivalent(readnoise.unit): + raise ValueError('Inconsistent units for gain ({}) '.format(gain.unit) + + ' ccd ({}) and readnoise ({}).'.format(ccd.unit, + readnoise.unit)) crmask, cleanarr = detect_cosmics( ccd.data, inmask=ccd.mask, sigclip=sigclip, sigfrac=sigfrac, objlim=objlim, gain=gain.value, diff --git a/ccdproc/tests/test_cosmicray.py b/ccdproc/tests/test_cosmicray.py index cf4eb4fd..8c87ee24 100644 --- a/ccdproc/tests/test_cosmicray.py +++ b/ccdproc/tests/test_cosmicray.py @@ -77,8 +77,11 @@ def test_cosmicray_gain_correct(array_input, gain_correct_data): add_cosmicrays(ccd_data, DATA_SCALE, threshold, ncrays=NCRAYS) noise = DATA_SCALE * np.ones_like(ccd_data.data) ccd_data.uncertainty = noise - # This may need units at some point. + # No units here on purpose. gain = 2.0 + # Don't really need to set this (6.5 is the default value) but want to + # make lack of units explicit. + readnoise = 6.5 if array_input: new_data, cr_mask = cosmicray_lacosmic(ccd_data.data, gain=gain, @@ -92,15 +95,16 @@ def test_cosmicray_gain_correct(array_input, gain_correct_data): # Fill masked locations with 0 since there is no simple relationship # between the original value and the corrected value. orig_data = np.ma.array(ccd_data.data, mask=cr_mask).filled(0) - new_data = np.ma.array(new_data, mask=cr_mask).filled(0) + new_data = np.ma.array(new_data.data, mask=cr_mask).filled(0) if gain_correct_data: gain_for_test = gain else: gain_for_test = 1.0 + np.testing.assert_allclose(gain_for_test * orig_data, new_data) -def test_cosmicray_lacosmic_accepts_quantity(): +def test_cosmicray_lacosmic_accepts_quantity_gain(): ccd_data = ccd_data_func(data_scale=DATA_SCALE) threshold = 5 add_cosmicrays(ccd_data, DATA_SCALE, threshold, ncrays=NCRAYS) @@ -108,6 +112,9 @@ def test_cosmicray_lacosmic_accepts_quantity(): ccd_data.uncertainty = noise # The units below are the point of the test gain = 2.0 * u.electron / u.adu + + # Since gain and ccd_data have units, the readnoise should too. + readnoise = 6.5 * u.electron new_ccd = cosmicray_lacosmic(ccd_data, gain=gain, gain_apply=True) @@ -119,7 +126,7 @@ def test_cosmicray_lacosmic_accepts_quantity_readnoise(): add_cosmicrays(ccd_data, DATA_SCALE, threshold, ncrays=NCRAYS) noise = DATA_SCALE * np.ones_like(ccd_data.data) ccd_data.uncertainty = noise - gain = 2.0 + gain = 2.0 * u.electron / u.adu # The units below are the point of this test readnoise = 6.5 * u.electron new_ccd = cosmicray_lacosmic(ccd_data, From 9af225273378ab762a2a21c8aa2d383ca715a6a1 Mon Sep 17 00:00:00 2001 From: Matthew Craig Date: Fri, 13 Dec 2019 10:17:24 -0700 Subject: [PATCH 09/15] Add changelog --- CHANGES.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGES.rst b/CHANGES.rst index 21bfac47..44ed861a 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -13,6 +13,8 @@ Other Changes and Additions Bug Fixes ^^^^^^^^^ +- Update units if gain is applied in `cosmicray_lacosmic`. [#716, #705] + 2.0.1 (2019-09-05) ------------------ From 44c794c5782fda014cf7c3b4be69875abee0908d Mon Sep 17 00:00:00 2001 From: Matthew Craig Date: Fri, 13 Dec 2019 10:43:03 -0700 Subject: [PATCH 10/15] Issue warning (instead of an exception) if input data is in electron --- ccdproc/core.py | 22 ++++++++++++++++------ ccdproc/tests/test_cosmicray.py | 23 +++++++++++++++++++++++ 2 files changed, 39 insertions(+), 6 deletions(-) diff --git a/ccdproc/core.py b/ccdproc/core.py index ff948b70..cb4c593f 100644 --- a/ccdproc/core.py +++ b/ccdproc/core.py @@ -5,6 +5,7 @@ import math import numbers import logging +import warnings import numpy as np from scipy import ndimage @@ -1494,12 +1495,21 @@ def cosmicray_lacosmic(ccd, sigclip=4.5, sigfrac=0.3, return cleanarr, crmask elif isinstance(ccd, CCDData): - # Check unit consistency before taking the time to check for - # cosmic rays. - if not (gain * ccd).unit.is_equivalent(readnoise.unit): - raise ValueError('Inconsistent units for gain ({}) '.format(gain.unit) + - ' ccd ({}) and readnoise ({}).'.format(ccd.unit, - readnoise.unit)) + # Start with a check for a special case: ccd is in electron, and + # gain and readnoise have no units. In that case we issue a warning + # instead of raising an error to avoid crashing user's pipelines. + if ccd.unit.is_equivalent(u.electron) and gain.value != 1.0: + warnings.warn("Image unit is electron but gain value " + "is not 1.0. Data maybe end up being gain " + "corrected twice.") + else: + # Check unit consistency before taking the time to check for + # cosmic rays. + if not (gain * ccd).unit.is_equivalent(readnoise.unit): + raise ValueError('Inconsistent units for gain ({}) '.format(gain.unit) + + ' ccd ({}) and readnoise ({}).'.format(ccd.unit, + readnoise.unit)) + crmask, cleanarr = detect_cosmics( ccd.data, inmask=ccd.mask, sigclip=sigclip, sigfrac=sigfrac, objlim=objlim, gain=gain.value, diff --git a/ccdproc/tests/test_cosmicray.py b/ccdproc/tests/test_cosmicray.py index 8c87ee24..4d9e7117 100644 --- a/ccdproc/tests/test_cosmicray.py +++ b/ccdproc/tests/test_cosmicray.py @@ -157,6 +157,29 @@ def test_cosmicray_lacosmic_detects_inconsistent_units(): assert 'Inconsistent units' in str(e.value) +def test_cosmicray_lacosmic_warns_on_ccd_in_electrons(recwarn): + # Check that an input ccd in electrons raises a warning. + ccd_data = ccd_data_func(data_scale=DATA_SCALE) + # The unit below is important for the test; this unit on + # input is supposed to raise an error. + ccd_data.unit = u.electron + threshold = 5 + add_cosmicrays(ccd_data, DATA_SCALE, threshold, ncrays=NCRAYS) + noise = DATA_SCALE * np.ones_like(ccd_data.data) + ccd_data.uncertainty = noise + # No units here on purpose. + gain = 2.0 + # Don't really need to set this (6.5 is the default value) but want to + # make lack of units explicit. + readnoise = 6.5 + new_ccd = cosmicray_lacosmic(ccd_data, + gain=gain, + gain_apply=True, + readnoise=readnoise) + assert len(recwarn) == 1 + assert "Image unit is electron" in str(recwarn.pop()) + + def test_cosmicray_median_check_data(): with pytest.raises(TypeError): ndata, crarr = cosmicray_median(10, thresh=5, mbox=11, From add755d6d850ae16e17f8512a0f1c4036c93a6b1 Mon Sep 17 00:00:00 2001 From: Matthew Craig Date: Fri, 13 Dec 2019 11:24:57 -0700 Subject: [PATCH 11/15] Add backticks in changelog --- CHANGES.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES.rst b/CHANGES.rst index 44ed861a..6a4de85f 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -13,7 +13,7 @@ Other Changes and Additions Bug Fixes ^^^^^^^^^ -- Update units if gain is applied in `cosmicray_lacosmic`. [#716, #705] +- Update units if gain is applied in ``cosmicray_lacosmic``. [#716, #705] 2.0.1 (2019-09-05) ------------------ From 7ac9cde286aee186cf910d41b9351276f5183641 Mon Sep 17 00:00:00 2001 From: Matthew Craig Date: Fri, 13 Dec 2019 13:19:59 -0700 Subject: [PATCH 12/15] Add logic to handle a special case when gain is one --- ccdproc/core.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/ccdproc/core.py b/ccdproc/core.py index cb4c593f..83e09ba0 100644 --- a/ccdproc/core.py +++ b/ccdproc/core.py @@ -1471,8 +1471,10 @@ def cosmicray_lacosmic(ccd, sigclip=4.5, sigfrac=0.3, # If we didn't get a quantity, put them in, with unit specified by the # documentation above. if not isinstance(gain, u.Quantity): + # Gain will change the value, so use the proper units gain = gain * u.electron / u.adu + # Set the units of readnoise to electrons, as specified in the # documentation, if no unit is present. if not isinstance(readnoise, u.Quantity): @@ -1502,7 +1504,12 @@ def cosmicray_lacosmic(ccd, sigclip=4.5, sigfrac=0.3, warnings.warn("Image unit is electron but gain value " "is not 1.0. Data maybe end up being gain " "corrected twice.") + else: + if ((readnoise.unit == u.electron) + and (ccd.unit == u.electron) + and (gain.value == 1.0)): + gain = gain.value * u.one # Check unit consistency before taking the time to check for # cosmic rays. if not (gain * ccd).unit.is_equivalent(readnoise.unit): From 3d76fafaa5d3a65d1b9a18304e1a054aff62db79 Mon Sep 17 00:00:00 2001 From: Matthew Craig Date: Fri, 13 Dec 2019 13:39:06 -0700 Subject: [PATCH 13/15] Fix PEP8 issues --- ccdproc/core.py | 1 - ccdproc/tests/test_cosmicray.py | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/ccdproc/core.py b/ccdproc/core.py index 83e09ba0..edaf39b3 100644 --- a/ccdproc/core.py +++ b/ccdproc/core.py @@ -1474,7 +1474,6 @@ def cosmicray_lacosmic(ccd, sigclip=4.5, sigfrac=0.3, # Gain will change the value, so use the proper units gain = gain * u.electron / u.adu - # Set the units of readnoise to electrons, as specified in the # documentation, if no unit is present. if not isinstance(readnoise, u.Quantity): diff --git a/ccdproc/tests/test_cosmicray.py b/ccdproc/tests/test_cosmicray.py index 4d9e7117..320f8f0d 100644 --- a/ccdproc/tests/test_cosmicray.py +++ b/ccdproc/tests/test_cosmicray.py @@ -140,7 +140,7 @@ def test_cosmicray_lacosmic_detects_inconsistent_units(): # of adu, a readnoise in electrons and a gain in adu / electron. # That is not internally inconsistent. ccd_data = ccd_data_func(data_scale=DATA_SCALE) - ccd_data.unit='adu' + ccd_data.unit = 'adu' threshold = 5 add_cosmicrays(ccd_data, DATA_SCALE, threshold, ncrays=NCRAYS) noise = DATA_SCALE * np.ones_like(ccd_data.data) From 00153b2f5f691ecc6121c86b82f661bf3f80bfe7 Mon Sep 17 00:00:00 2001 From: Matt Craig Date: Sun, 22 Dec 2019 13:22:25 -0600 Subject: [PATCH 14/15] Add documentation of change to lacosmic --- ccdproc/core.py | 16 ++++++++++------ docs/reduction_toolbox.rst | 16 ++++++++++++++++ 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/ccdproc/core.py b/ccdproc/core.py index edaf39b3..f21bfb34 100644 --- a/ccdproc/core.py +++ b/ccdproc/core.py @@ -1316,11 +1316,18 @@ def cosmicray_lacosmic(ccd, sigclip=4.5, sigfrac=0.3, code describe in van Dokkum (2001) [1]_ as implemented by McCully (2014) [2]_. If you use this algorithm, please cite these two works. + + Parameters ---------- ccd : `~astropy.nddata.CCDData` or `numpy.ndarray` Data to have cosmic ray cleaned. + gain_apply : bool, optional + If ``True``, **return gain-corrected data**, with correct units, + otherwise do not gain-correct the data. Default is ``True`` to + preserve backwards compatibility. + sigclip : float, optional Laplacian-to-noise limit for cosmic ray detection. Lower values will flag more pixels as cosmic rays. Default: 4.5. @@ -1344,11 +1351,6 @@ def cosmicray_lacosmic(ccd, sigclip=4.5, sigfrac=0.3, Gain of the image (electrons / ADU). We always need to work in electrons for cosmic ray detection. Default: 1.0 - gain_apply : bool, optional - If ``True``, return gain-corrected data, with correct units, otherwise - do not gain-correct the data. Default is ``True`` to preserve - backwards compatibility. - readnoise : float, optional Read noise of the image (electrons). Used to generate the noise model of the image. Default: 6.5. @@ -1429,7 +1431,9 @@ def cosmicray_lacosmic(ccd, sigclip=4.5, sigfrac=0.3, nccd : `~astropy.nddata.CCDData` or `numpy.ndarray` An object of the same type as ccd is returned. If it is a `~astropy.nddata.CCDData`, the mask attribute will also be updated with - areas identified with cosmic rays masked. + areas identified with cosmic rays masked. **By default, the image is + multiplied by the gain.** You can control this behavior with the + ``gain_apply`` argument. crmask : `numpy.ndarray` If an `numpy.ndarray` is provided as ccd, a boolean ndarray with the diff --git a/docs/reduction_toolbox.rst b/docs/reduction_toolbox.rst index 9cfc6bf3..d8a837a1 100644 --- a/docs/reduction_toolbox.rst +++ b/docs/reduction_toolbox.rst @@ -99,6 +99,22 @@ Use this technique with `~ccdproc.cosmicray_lacosmic`: >>> cr_cleaned = ccdproc.cosmicray_lacosmic(gain_corrected, sigclip=5) +.. note:: + + A significant error in `~ccdproc.cosmicray_lacosmic` in version 2.1 of + ccdproc. By default, `~ccdproc.cosmicray_lacosmic` multiplies the image by + the gain; prior to version 2.1 it did so without changing the units of + the image. + + There are two ways to correctly invoke `~ccdproc.cosmicray_lacosmic`: + + + Supply a gain-corrected image, in units of ``electron``, and set ``gain=1.0`` + (the default value) in `~ccdproc.cosmicray_lacosmic`. + + Supply an image in ``adu`` and set the ``gain`` argument of + `~ccdproc.cosmicray_lacosmic` to the appropriate value for your + instrument. Ideally, pass in a ``gain`` with units, but if units are + omitted the will be assumed to be ``electron/adu``. + median ++++++ From a9503a7fb782fb76e58c06b9be124f8561adf423 Mon Sep 17 00:00:00 2001 From: Matt Craig Date: Tue, 24 Dec 2019 13:22:03 -0600 Subject: [PATCH 15/15] Modify documentation note in response to review --- docs/reduction_toolbox.rst | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/docs/reduction_toolbox.rst b/docs/reduction_toolbox.rst index d8a837a1..8be96dc8 100644 --- a/docs/reduction_toolbox.rst +++ b/docs/reduction_toolbox.rst @@ -101,10 +101,9 @@ Use this technique with `~ccdproc.cosmicray_lacosmic`: .. note:: - A significant error in `~ccdproc.cosmicray_lacosmic` in version 2.1 of - ccdproc. By default, `~ccdproc.cosmicray_lacosmic` multiplies the image by + By default, `~ccdproc.cosmicray_lacosmic` multiplies the image by the gain; prior to version 2.1 it did so without changing the units of - the image. + the image which could result in incorrect results. There are two ways to correctly invoke `~ccdproc.cosmicray_lacosmic`: