From e20ad24cbfe3fc14e74364d040b60f0eca586f19 Mon Sep 17 00:00:00 2001 From: Eric Koch Date: Tue, 1 Oct 2024 09:17:06 -0400 Subject: [PATCH 01/10] Add fix for astropy #17096 (https://github.com/astropy/astropy/issues/17096) --- radio_beam/beam.py | 50 ++++++++++++++++++++++------------------------ 1 file changed, 24 insertions(+), 26 deletions(-) diff --git a/radio_beam/beam.py b/radio_beam/beam.py index 1fa41bd..d8d8a7d 100644 --- a/radio_beam/beam.py +++ b/radio_beam/beam.py @@ -27,13 +27,23 @@ def _to_area(major,minor): u.arcmin: "'"} +def _set_default_unit(type_str, value, unit, equiv_unit=u.deg): + if not hasattr(value, 'unit'): + return value * unit + + if value.unit.is_equivalent(equiv_unit): + return value + else: + raise u.UnitsError(f"{value.unit} for {type_str} is not equivalent to {equiv_unit}") + + class Beam(u.Quantity): """ An object to handle single radio beams. """ def __new__(cls, major=None, minor=None, pa=None, area=None, - default_unit=u.arcsec, meta=None): + default_unit=u.deg, meta=None): """ Create a new Gaussian beam @@ -51,6 +61,8 @@ def __new__(cls, major=None, minor=None, pa=None, area=None, Gaussian beam. default_unit : :class:`~astropy.units.Unit` The unit to impose on major, minor if they are specified as floats + meta : dict, optional + A dictionary of metadata to store with the beam. """ # improve to some kwargs magic later @@ -69,34 +81,20 @@ def __new__(cls, major=None, minor=None, pa=None, area=None, minor = rad * SIGMA_TO_FWHM pa = 0.0 * u.deg - # give specified values priority - if major is not None: - if u.deg.is_equivalent(major): - major = major - else: - warnings.warn("Assuming major axis has been specified in degrees") - major = major * u.deg - if minor is not None: - if u.deg.is_equivalent(minor): - minor = minor - else: - warnings.warn("Assuming minor axis has been specified in degrees") - minor = minor * u.deg - if pa is not None: - if u.deg.is_equivalent(pa): - pa = pa - else: - warnings.warn("Assuming position angle has been specified in degrees") - pa = pa * u.deg else: - pa = 0.0 * u.deg + # give specified values priority - # some sensible defaults - if minor is None: - minor = major + major = _set_default_unit("major", major, default_unit, equiv_unit=u.deg) + pa = _set_default_unit("pa", pa, default_unit, equiv_unit=u.deg) + + # some sensible defaults + if minor is None: + minor = major + else: + minor = _set_default_unit("minor", minor, default_unit, equiv_unit=u.deg) - if minor > major: - raise ValueError("Minor axis greater than major axis.") + if minor > major: + raise ValueError("Minor axis greater than major axis.") self = super(Beam, cls).__new__(cls, _to_area(major,minor).value, u.sr) self._major = major From 7bb8e7e56800385aa07ea636c611a56925aa0948 Mon Sep 17 00:00:00 2001 From: Eric Koch Date: Tue, 1 Oct 2024 09:22:31 -0400 Subject: [PATCH 02/10] Restore to previous logic and order in setting params --- radio_beam/beam.py | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/radio_beam/beam.py b/radio_beam/beam.py index d8d8a7d..49705b0 100644 --- a/radio_beam/beam.py +++ b/radio_beam/beam.py @@ -81,20 +81,23 @@ def __new__(cls, major=None, minor=None, pa=None, area=None, minor = rad * SIGMA_TO_FWHM pa = 0.0 * u.deg - else: - # give specified values priority - + # give specified values priority + if major is not None: major = _set_default_unit("major", major, default_unit, equiv_unit=u.deg) + + if pa is not None: pa = _set_default_unit("pa", pa, default_unit, equiv_unit=u.deg) + else: + pa = 0 * u.deg - # some sensible defaults - if minor is None: - minor = major - else: - minor = _set_default_unit("minor", minor, default_unit, equiv_unit=u.deg) + # some sensible defaults + if minor is None: + minor = major + else: + minor = _set_default_unit("minor", minor, default_unit, equiv_unit=u.deg) - if minor > major: - raise ValueError("Minor axis greater than major axis.") + if minor > major: + raise ValueError("Minor axis greater than major axis.") self = super(Beam, cls).__new__(cls, _to_area(major,minor).value, u.sr) self._major = major From 1fa730f0cf79124cfd71065381872a4fd1627e85 Mon Sep 17 00:00:00 2001 From: Eric Koch Date: Tue, 1 Oct 2024 09:25:57 -0400 Subject: [PATCH 03/10] pa units default to deg --- radio_beam/beam.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/radio_beam/beam.py b/radio_beam/beam.py index 49705b0..6de59e3 100644 --- a/radio_beam/beam.py +++ b/radio_beam/beam.py @@ -86,7 +86,7 @@ def __new__(cls, major=None, minor=None, pa=None, area=None, major = _set_default_unit("major", major, default_unit, equiv_unit=u.deg) if pa is not None: - pa = _set_default_unit("pa", pa, default_unit, equiv_unit=u.deg) + pa = _set_default_unit("pa", pa, u.deg, equiv_unit=u.deg) else: pa = 0 * u.deg From 35ec3bfa7c37a06b2bcab24608c3b2e8b6b813b2 Mon Sep 17 00:00:00 2001 From: Eric Koch Date: Tue, 1 Oct 2024 09:28:17 -0400 Subject: [PATCH 04/10] Apply fix to radio_beam.Beams --- radio_beam/multiple_beams.py | 30 +++++++++++------------------- 1 file changed, 11 insertions(+), 19 deletions(-) diff --git a/radio_beam/multiple_beams.py b/radio_beam/multiple_beams.py index 56b149b..bac4af2 100644 --- a/radio_beam/multiple_beams.py +++ b/radio_beam/multiple_beams.py @@ -6,7 +6,7 @@ import numpy as np import warnings -from .beam import Beam, _to_area, SIGMA_TO_FWHM +from .beam import Beam, _to_area, SIGMA_TO_FWHM, _set_default_unit from .commonbeam import commonbeam from .utils import InvalidBeamOperationError @@ -30,12 +30,14 @@ def __new__(cls, major=None, minor=None, pa=None, The FWHM minor axes pa : :class:`~astropy.units.Quantity` with angular equivalency The beam position angles - area : :class:`~astropy.units.Quantity` with steradian equivalency + areas : :class:`~astropy.units.Quantity` with steradian equivalency The area of the beams. This is an alternative to specifying the major/minor/PA, and will create those values assuming a circular Gaussian beam. default_unit : :class:`~astropy.units.Unit` The unit to impose on major, minor if they are specified as floats + meta : dict, optional + A dictionary of metadata to include in the header. beams : List of :class:`~radio_beam.Beam` objects List of individual `Beam` objects. The resulting `Beams` object will have major and minor axes in degrees. @@ -59,33 +61,23 @@ def __new__(cls, major=None, minor=None, pa=None, # give specified values priority if major is not None: - if u.deg.is_equivalent(major.unit): - pass - else: - warnings.warn("Assuming major axes has been specified in degrees") - major = major * u.deg - if minor is not None: - if u.deg.is_equivalent(minor.unit): - pass - else: - warnings.warn("Assuming minor axes has been specified in degrees") - minor = minor * u.deg + major = _set_default_unit("major", major, default_unit, equiv_unit=u.deg) + + if pa is not None: if len(pa) != len(major): raise ValueError("Number of position angles must match number of major axis lengths") - if u.deg.is_equivalent(pa.unit): - pass - else: - warnings.warn("Assuming position angles has been specified in degrees") - pa = pa * u.deg + pa = _set_default_unit("pa", pa, u.deg, equiv_unit=u.deg) else: - pa = np.zeros_like(major.value) * u.deg + pa = np.zeros(major.shape) * u.deg # some sensible defaults if minor is None: minor = major elif len(minor) != len(major): raise ValueError("Minor and major axes must have same number of values") + else: + minor = _set_default_unit("minor", minor, default_unit, equiv_unit=u.deg) if np.any(minor > major): raise ValueError("Minor axis greater than major axis.") From e59016a7fe0508a709a44bce48adbc81b90dccca Mon Sep 17 00:00:00 2001 From: Eric Koch Date: Tue, 1 Oct 2024 09:31:58 -0400 Subject: [PATCH 05/10] Keep default units to deg (since we weren't using this consistently beforehand) --- radio_beam/multiple_beams.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/radio_beam/multiple_beams.py b/radio_beam/multiple_beams.py index bac4af2..d6fc834 100644 --- a/radio_beam/multiple_beams.py +++ b/radio_beam/multiple_beams.py @@ -17,7 +17,7 @@ class Beams(u.Quantity): """ def __new__(cls, major=None, minor=None, pa=None, - areas=None, default_unit=u.arcsec, meta=None, + areas=None, default_unit=u.deg, meta=None, beams=None): """ Create a new set of Gaussian beams From 19e73285b6ae05e2e861c7aacde8ea0c89a2b389 Mon Sep 17 00:00:00 2001 From: Eric Koch Date: Tue, 1 Oct 2024 09:35:19 -0400 Subject: [PATCH 06/10] Remove np.NaN use --- radio_beam/tests/test_beams.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/radio_beam/tests/test_beams.py b/radio_beam/tests/test_beams.py index c09f54f..008e398 100644 --- a/radio_beam/tests/test_beams.py +++ b/radio_beam/tests/test_beams.py @@ -321,7 +321,7 @@ def test_extrema_beams(beams, majors, minors, pas): assert extrema[1].minor.value == minors[mask].max().value -@pytest.mark.parametrize("majors", [[1, 1, 1, 2, np.NaN, 4], +@pytest.mark.parametrize("majors", [[1, 1, 1, 2, np.nan, 4], [0, 1, 1, 2, 3, 4]]) def test_beams_with_invalid(majors): From d6d20aa743679b8f1d54841f285f01d73d8a07d5 Mon Sep 17 00:00:00 2001 From: Eric Koch Date: Tue, 1 Oct 2024 09:41:57 -0400 Subject: [PATCH 07/10] Restore defaultunit to arcsec --- radio_beam/beam.py | 2 +- radio_beam/multiple_beams.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/radio_beam/beam.py b/radio_beam/beam.py index 6de59e3..e46c8a8 100644 --- a/radio_beam/beam.py +++ b/radio_beam/beam.py @@ -43,7 +43,7 @@ class Beam(u.Quantity): """ def __new__(cls, major=None, minor=None, pa=None, area=None, - default_unit=u.deg, meta=None): + default_unit=u.arcsec, meta=None): """ Create a new Gaussian beam diff --git a/radio_beam/multiple_beams.py b/radio_beam/multiple_beams.py index d6fc834..bac4af2 100644 --- a/radio_beam/multiple_beams.py +++ b/radio_beam/multiple_beams.py @@ -17,7 +17,7 @@ class Beams(u.Quantity): """ def __new__(cls, major=None, minor=None, pa=None, - areas=None, default_unit=u.deg, meta=None, + areas=None, default_unit=u.arcsec, meta=None, beams=None): """ Create a new set of Gaussian beams From 14e125db286b9749df492992b513b73df8e77571 Mon Sep 17 00:00:00 2001 From: Eric Koch Date: Tue, 1 Oct 2024 09:43:23 -0400 Subject: [PATCH 08/10] Consistent use of default_unit and u.deg in tests --- radio_beam/tests/test_beam.py | 2 +- radio_beam/tests/test_kernels.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/radio_beam/tests/test_beam.py b/radio_beam/tests/test_beam.py index 222bdea..1a05c2d 100644 --- a/radio_beam/tests/test_beam.py +++ b/radio_beam/tests/test_beam.py @@ -62,7 +62,7 @@ def test_fits_from_casa(): def test_manual(): # Instantiate from command line - man_beam_val = Beam(0.1, 0.1, 30) + man_beam_val = Beam(0.1, 0.1, 30, default_unit=u.deg) npt.assert_almost_equal(man_beam_val.value, 3.451589629868801e-06) man_beam_rad = Beam(0.1*u.rad, 0.1*u.rad, 30*u.deg) npt.assert_almost_equal(man_beam_rad.value, 0.011330900354567986) diff --git a/radio_beam/tests/test_kernels.py b/radio_beam/tests/test_kernels.py index ef74cbc..1b125e8 100644 --- a/radio_beam/tests/test_kernels.py +++ b/radio_beam/tests/test_kernels.py @@ -16,7 +16,7 @@ reason="Must have astropy version >1.1") def test_gauss_kernel(): - fake_beam = radio_beam.Beam(10) + fake_beam = radio_beam.Beam(10*u.deg) # Let pixscale be 0.1 deg/pix kernel = fake_beam.as_kernel(0.1*u.deg) @@ -33,7 +33,7 @@ def test_gauss_kernel(): reason="Must have astropy version >1.1") def test_tophat_kernel(): - fake_beam = radio_beam.Beam(10) + fake_beam = radio_beam.Beam(10*u.deg) # Let pixscale be 0.1 deg/pix kernel = fake_beam.as_tophat_kernel(0.1*u.deg) From ad0e8bde15b0c7a38b8c21f7643330d5011298e8 Mon Sep 17 00:00:00 2001 From: Eric Koch Date: Tue, 1 Oct 2024 10:14:57 -0400 Subject: [PATCH 09/10] Update name to _with_default_unit --- radio_beam/beam.py | 8 ++++---- radio_beam/multiple_beams.py | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/radio_beam/beam.py b/radio_beam/beam.py index e46c8a8..423f7c6 100644 --- a/radio_beam/beam.py +++ b/radio_beam/beam.py @@ -27,7 +27,7 @@ def _to_area(major,minor): u.arcmin: "'"} -def _set_default_unit(type_str, value, unit, equiv_unit=u.deg): +def _with_default_unit(type_str, value, unit, equiv_unit=u.deg): if not hasattr(value, 'unit'): return value * unit @@ -83,10 +83,10 @@ def __new__(cls, major=None, minor=None, pa=None, area=None, # give specified values priority if major is not None: - major = _set_default_unit("major", major, default_unit, equiv_unit=u.deg) + major = _with_default_unit("major", major, default_unit, equiv_unit=u.deg) if pa is not None: - pa = _set_default_unit("pa", pa, u.deg, equiv_unit=u.deg) + pa = _with_default_unit("pa", pa, u.deg, equiv_unit=u.deg) else: pa = 0 * u.deg @@ -94,7 +94,7 @@ def __new__(cls, major=None, minor=None, pa=None, area=None, if minor is None: minor = major else: - minor = _set_default_unit("minor", minor, default_unit, equiv_unit=u.deg) + minor = _with_default_unit("minor", minor, default_unit, equiv_unit=u.deg) if minor > major: raise ValueError("Minor axis greater than major axis.") diff --git a/radio_beam/multiple_beams.py b/radio_beam/multiple_beams.py index bac4af2..45e7080 100644 --- a/radio_beam/multiple_beams.py +++ b/radio_beam/multiple_beams.py @@ -6,7 +6,7 @@ import numpy as np import warnings -from .beam import Beam, _to_area, SIGMA_TO_FWHM, _set_default_unit +from .beam import Beam, _to_area, SIGMA_TO_FWHM, _with_default_unit from .commonbeam import commonbeam from .utils import InvalidBeamOperationError @@ -61,13 +61,13 @@ def __new__(cls, major=None, minor=None, pa=None, # give specified values priority if major is not None: - major = _set_default_unit("major", major, default_unit, equiv_unit=u.deg) + major = _with_default_unit("major", major, default_unit, equiv_unit=u.deg) if pa is not None: if len(pa) != len(major): raise ValueError("Number of position angles must match number of major axis lengths") - pa = _set_default_unit("pa", pa, u.deg, equiv_unit=u.deg) + pa = _with_default_unit("pa", pa, u.deg, equiv_unit=u.deg) else: pa = np.zeros(major.shape) * u.deg @@ -77,7 +77,7 @@ def __new__(cls, major=None, minor=None, pa=None, elif len(minor) != len(major): raise ValueError("Minor and major axes must have same number of values") else: - minor = _set_default_unit("minor", minor, default_unit, equiv_unit=u.deg) + minor = _with_default_unit("minor", minor, default_unit, equiv_unit=u.deg) if np.any(minor > major): raise ValueError("Minor axis greater than major axis.") From 9cb9a82ca99eec747e55897152052b96faab6719 Mon Sep 17 00:00:00 2001 From: Eric Koch Date: Fri, 18 Oct 2024 09:36:34 -0400 Subject: [PATCH 10/10] Clean-up unneeded equiv_unit --- radio_beam/beam.py | 10 +++++----- radio_beam/multiple_beams.py | 6 +++--- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/radio_beam/beam.py b/radio_beam/beam.py index 423f7c6..9955236 100644 --- a/radio_beam/beam.py +++ b/radio_beam/beam.py @@ -27,11 +27,11 @@ def _to_area(major,minor): u.arcmin: "'"} -def _with_default_unit(type_str, value, unit, equiv_unit=u.deg): +def _with_default_unit(type_str, value, unit): if not hasattr(value, 'unit'): return value * unit - if value.unit.is_equivalent(equiv_unit): + if value.unit.is_equivalent(unit): return value else: raise u.UnitsError(f"{value.unit} for {type_str} is not equivalent to {equiv_unit}") @@ -83,10 +83,10 @@ def __new__(cls, major=None, minor=None, pa=None, area=None, # give specified values priority if major is not None: - major = _with_default_unit("major", major, default_unit, equiv_unit=u.deg) + major = _with_default_unit("major", major, default_unit) if pa is not None: - pa = _with_default_unit("pa", pa, u.deg, equiv_unit=u.deg) + pa = _with_default_unit("pa", pa, u.deg) else: pa = 0 * u.deg @@ -94,7 +94,7 @@ def __new__(cls, major=None, minor=None, pa=None, area=None, if minor is None: minor = major else: - minor = _with_default_unit("minor", minor, default_unit, equiv_unit=u.deg) + minor = _with_default_unit("minor", minor, default_unit) if minor > major: raise ValueError("Minor axis greater than major axis.") diff --git a/radio_beam/multiple_beams.py b/radio_beam/multiple_beams.py index 45e7080..6561162 100644 --- a/radio_beam/multiple_beams.py +++ b/radio_beam/multiple_beams.py @@ -61,13 +61,13 @@ def __new__(cls, major=None, minor=None, pa=None, # give specified values priority if major is not None: - major = _with_default_unit("major", major, default_unit, equiv_unit=u.deg) + major = _with_default_unit("major", major, default_unit) if pa is not None: if len(pa) != len(major): raise ValueError("Number of position angles must match number of major axis lengths") - pa = _with_default_unit("pa", pa, u.deg, equiv_unit=u.deg) + pa = _with_default_unit("pa", pa, u.deg) else: pa = np.zeros(major.shape) * u.deg @@ -77,7 +77,7 @@ def __new__(cls, major=None, minor=None, pa=None, elif len(minor) != len(major): raise ValueError("Minor and major axes must have same number of values") else: - minor = _with_default_unit("minor", minor, default_unit, equiv_unit=u.deg) + minor = _with_default_unit("minor", minor, default_unit) if np.any(minor > major): raise ValueError("Minor axis greater than major axis.")