From e2b543c0c5a931630275fdfd9e844413d3a78c56 Mon Sep 17 00:00:00 2001 From: "Raphael A. P. Oliveira" Date: Thu, 4 Jan 2024 18:13:58 +0100 Subject: [PATCH 1/7] Forcing Astropy SkyCoord input to be equatorial, issue #70 --- source/MulensModel/coordinates.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/source/MulensModel/coordinates.py b/source/MulensModel/coordinates.py index 8bd6e42a..6dd4b86f 100644 --- a/source/MulensModel/coordinates.py +++ b/source/MulensModel/coordinates.py @@ -36,10 +36,22 @@ class Coordinates(SkyCoord): """ def __init__(self, *args, **kwargs): - if not isinstance(args[0], (SkyCoord, u.quantity.Quantity)): + if isinstance(args[0], str): if 'unit' not in kwargs and len(args) > 0: self._check_for_ra_in_degrees(args[0]) kwargs['unit'] = (u.hourangle, u.deg) + if kwargs.get('frame') not in [None, 'icrs', 'fk4', 'fk5']: + raise ValueError("Only ICRS, FK4 and FK5 frames are allowed" + + " to Coordinates().") + + elif isinstance(args[0], (SkyCoord, u.quantity.Quantity)): + test = '18h00m00s -30d00m00s' + is_icrs = args[0].is_equivalent_frame(SkyCoord(test, frame='icrs')) + is_fk4 = args[0].is_equivalent_frame(SkyCoord(test, frame='fk4')) + is_fk5 = args[0].is_equivalent_frame(SkyCoord(test, frame='fk5')) + if not (is_icrs | is_fk4 | is_fk5): + raise ValueError("Provided SkyCoord is not in allowed frame.") + SkyCoord.__init__(self, *args, **kwargs) if self.cartesian.xyz.shape not in [(3,), (3, 1)]: raise ValueError( From 2aa38cc42713a0be8677818e5fc75460d93070b0 Mon Sep 17 00:00:00 2001 From: "Raphael A. P. Oliveira" Date: Thu, 11 Jan 2024 16:28:08 +0100 Subject: [PATCH 2/7] New function to validate coordinates input, check test failure --- source/MulensModel/coordinates.py | 38 +++++++++++++++++++++---------- 1 file changed, 26 insertions(+), 12 deletions(-) diff --git a/source/MulensModel/coordinates.py b/source/MulensModel/coordinates.py index 6dd4b86f..dd4b97ce 100644 --- a/source/MulensModel/coordinates.py +++ b/source/MulensModel/coordinates.py @@ -1,7 +1,7 @@ import numpy as np import warnings -from astropy.coordinates import SkyCoord +from astropy.coordinates import SkyCoord, ICRS, FK4, FK5 from astropy import units as u from MulensModel.utils import Utils @@ -40,17 +40,7 @@ def __init__(self, *args, **kwargs): if 'unit' not in kwargs and len(args) > 0: self._check_for_ra_in_degrees(args[0]) kwargs['unit'] = (u.hourangle, u.deg) - if kwargs.get('frame') not in [None, 'icrs', 'fk4', 'fk5']: - raise ValueError("Only ICRS, FK4 and FK5 frames are allowed" + - " to Coordinates().") - - elif isinstance(args[0], (SkyCoord, u.quantity.Quantity)): - test = '18h00m00s -30d00m00s' - is_icrs = args[0].is_equivalent_frame(SkyCoord(test, frame='icrs')) - is_fk4 = args[0].is_equivalent_frame(SkyCoord(test, frame='fk4')) - is_fk5 = args[0].is_equivalent_frame(SkyCoord(test, frame='fk5')) - if not (is_icrs | is_fk4 | is_fk5): - raise ValueError("Provided SkyCoord is not in allowed frame.") + self._validate_input(args[0], kwargs.get('frame')) SkyCoord.__init__(self, *args, **kwargs) if self.cartesian.xyz.shape not in [(3,), (3, 1)]: @@ -85,6 +75,30 @@ def _check_for_ra_in_degrees(self, arg): "a default unit for RA is hours (not degrees). " + str(value)) warnings.warn(warning, UserWarning) + def _validate_input(self, arg, frame): + """ + Validate input for coordinates, checking if format is allowed (ICRS, + FK4 or FK5) or raising ValueError otherwise. If SkyCoord() instance + is provided, the frame should be allowed as well. + """ + allowed_fmts = (str, SkyCoord, ICRS, FK4, FK5) + if not isinstance(arg, allowed_fmts): + class_ = type(arg) + raise ValueError(f'Coordinate format {class_} is not allowed.') + + if isinstance(arg, str): + if frame not in [None, 'icrs', 'fk4', 'fk5']: + raise ValueError("Only ICRS, FK4 and FK5 frames are allowed" + + " to Coordinates().") + + elif isinstance(arg, (SkyCoord, u.quantity.Quantity)): + test = '18h00m00s -30d00m00s' + is_icrs = arg.is_equivalent_frame(SkyCoord(test, frame='icrs')) + is_fk4 = arg.is_equivalent_frame(SkyCoord(test, frame='fk4')) + is_fk5 = arg.is_equivalent_frame(SkyCoord(test, frame='fk5')) + if not (is_icrs | is_fk4 | is_fk5): + raise ValueError("Provided SkyCoord is not in allowed frame.") + def _calculate_projected(self): """ Calculate North and East directions projected on the plane of the sky. From f8d1994e58b6a3c8637a9c6f1a3769aeba787a65 Mon Sep 17 00:00:00 2001 From: "Raphael A. P. Oliveira" Date: Thu, 11 Jan 2024 18:41:07 +0100 Subject: [PATCH 3/7] Added usr argument and unit tests, issue #70 --- source/MulensModel/coordinates.py | 10 +++++----- source/MulensModel/tests/test_Coords.py | 22 +++++++++++++++++++++- 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/source/MulensModel/coordinates.py b/source/MulensModel/coordinates.py index dd4b97ce..5c3241b5 100644 --- a/source/MulensModel/coordinates.py +++ b/source/MulensModel/coordinates.py @@ -40,7 +40,8 @@ def __init__(self, *args, **kwargs): if 'unit' not in kwargs and len(args) > 0: self._check_for_ra_in_degrees(args[0]) kwargs['unit'] = (u.hourangle, u.deg) - self._validate_input(args[0], kwargs.get('frame')) + frame, usr = kwargs.get('frame'), kwargs.pop('usr', False) + self._validate_input(args[0], frame, usr) SkyCoord.__init__(self, *args, **kwargs) if self.cartesian.xyz.shape not in [(3,), (3, 1)]: @@ -75,16 +76,15 @@ def _check_for_ra_in_degrees(self, arg): "a default unit for RA is hours (not degrees). " + str(value)) warnings.warn(warning, UserWarning) - def _validate_input(self, arg, frame): + def _validate_input(self, arg, frame, usr): """ Validate input for coordinates, checking if format is allowed (ICRS, FK4 or FK5) or raising ValueError otherwise. If SkyCoord() instance is provided, the frame should be allowed as well. """ allowed_fmts = (str, SkyCoord, ICRS, FK4, FK5) - if not isinstance(arg, allowed_fmts): - class_ = type(arg) - raise ValueError(f'Coordinate format {class_} is not allowed.') + if usr and not isinstance(arg, allowed_fmts): + raise ValueError(f'Coordinate format {type(arg)} is not allowed.') if isinstance(arg, str): if frame not in [None, 'icrs', 'fk4', 'fk5']: diff --git a/source/MulensModel/tests/test_Coords.py b/source/MulensModel/tests/test_Coords.py index 23c1dee6..a9713876 100644 --- a/source/MulensModel/tests/test_Coords.py +++ b/source/MulensModel/tests/test_Coords.py @@ -1,6 +1,7 @@ import os +import unittest import numpy as np -from astropy.coordinates import SkyCoord +from astropy.coordinates import SkyCoord, ICRS, FK4, FK5, Galactic import astropy.units as u import MulensModel as mm @@ -49,6 +50,25 @@ def test_event_coords(): assert event_3.model.coords.to_string('hmsdms') == new_coord_str +def test_coord_validation(): + + c_test = '18h00m00s -30d00m00s' + tests = [mm.Coordinates(c_test, usr=True), + mm.Coordinates(c_test, frame='fk4', usr=True), + mm.Coordinates(SkyCoord(c_test, frame='fk5'), usr=True), + mm.Coordinates(ICRS(*c_test.split()), usr=True), + mm.Coordinates(FK4(*c_test.split()), usr=True), + mm.Coordinates(FK5(*c_test.split()), usr=True)] + t_errors = [(c_test, 'galactic'), + (SkyCoord(c_test, frame='galactic'), None), + (Galactic(*c_test.split()), None)] + for test in tests: + assert test.to_string('hmsdms') == c_test + for error in t_errors: + with unittest.TestCase().assertRaises(ValueError): + mm.Coordinates(error[0], frame=error[1], usr=True) + + def check_event_coords(event, ra, dec): """ For given Event instance event, check if .ra, .model.ra, From 1d70a08fa233f3c85bd8116cfbedebd643fcb0f9 Mon Sep 17 00:00:00 2001 From: radek_poleski Date: Wed, 17 Jan 2024 21:28:49 +0100 Subject: [PATCH 4/7] reverting addintion of usr kwarg in Coordinates; 1 test fails --- source/MulensModel/coordinates.py | 8 ++++---- source/MulensModel/tests/test_Coords.py | 15 +++++++-------- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/source/MulensModel/coordinates.py b/source/MulensModel/coordinates.py index 5c3241b5..be8c474e 100644 --- a/source/MulensModel/coordinates.py +++ b/source/MulensModel/coordinates.py @@ -40,8 +40,8 @@ def __init__(self, *args, **kwargs): if 'unit' not in kwargs and len(args) > 0: self._check_for_ra_in_degrees(args[0]) kwargs['unit'] = (u.hourangle, u.deg) - frame, usr = kwargs.get('frame'), kwargs.pop('usr', False) - self._validate_input(args[0], frame, usr) + frame = kwargs.get('frame') + self._validate_input(args[0], frame) SkyCoord.__init__(self, *args, **kwargs) if self.cartesian.xyz.shape not in [(3,), (3, 1)]: @@ -76,14 +76,14 @@ def _check_for_ra_in_degrees(self, arg): "a default unit for RA is hours (not degrees). " + str(value)) warnings.warn(warning, UserWarning) - def _validate_input(self, arg, frame, usr): + def _validate_input(self, arg, frame): """ Validate input for coordinates, checking if format is allowed (ICRS, FK4 or FK5) or raising ValueError otherwise. If SkyCoord() instance is provided, the frame should be allowed as well. """ allowed_fmts = (str, SkyCoord, ICRS, FK4, FK5) - if usr and not isinstance(arg, allowed_fmts): + if not isinstance(arg, allowed_fmts): raise ValueError(f'Coordinate format {type(arg)} is not allowed.') if isinstance(arg, str): diff --git a/source/MulensModel/tests/test_Coords.py b/source/MulensModel/tests/test_Coords.py index a9713876..a91b2490 100644 --- a/source/MulensModel/tests/test_Coords.py +++ b/source/MulensModel/tests/test_Coords.py @@ -53,12 +53,12 @@ def test_event_coords(): def test_coord_validation(): c_test = '18h00m00s -30d00m00s' - tests = [mm.Coordinates(c_test, usr=True), - mm.Coordinates(c_test, frame='fk4', usr=True), - mm.Coordinates(SkyCoord(c_test, frame='fk5'), usr=True), - mm.Coordinates(ICRS(*c_test.split()), usr=True), - mm.Coordinates(FK4(*c_test.split()), usr=True), - mm.Coordinates(FK5(*c_test.split()), usr=True)] + tests = [mm.Coordinates(c_test), + mm.Coordinates(c_test, frame='fk4'), + mm.Coordinates(SkyCoord(c_test, frame='fk5')), + mm.Coordinates(ICRS(*c_test.split())), + mm.Coordinates(FK4(*c_test.split())), + mm.Coordinates(FK5(*c_test.split()))] t_errors = [(c_test, 'galactic'), (SkyCoord(c_test, frame='galactic'), None), (Galactic(*c_test.split()), None)] @@ -66,8 +66,7 @@ def test_coord_validation(): assert test.to_string('hmsdms') == c_test for error in t_errors: with unittest.TestCase().assertRaises(ValueError): - mm.Coordinates(error[0], frame=error[1], usr=True) - + mm.Coordinates(error[0], frame=error[1]) def check_event_coords(event, ra, dec): """ From 24699729fc45bd8007cfc60fd5be2d2f6c3a098e Mon Sep 17 00:00:00 2001 From: radek_poleski Date: Wed, 17 Jan 2024 21:30:46 +0100 Subject: [PATCH 5/7] correcting galactic_X in Coordinates; still 1 test fails, but in different place --- source/MulensModel/coordinates.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source/MulensModel/coordinates.py b/source/MulensModel/coordinates.py index be8c474e..e422448d 100644 --- a/source/MulensModel/coordinates.py +++ b/source/MulensModel/coordinates.py @@ -122,7 +122,7 @@ def galactic_l(self): Galactic longitude. Note that for connivance, the values l > 180 degrees are represented as 360-l. """ - gal_l = self.galactic.l + gal_l = SkyCoord(self).galactic.l if gal_l > 180. * u.deg: gal_l = gal_l - 360. * u.deg return gal_l @@ -134,7 +134,7 @@ def galactic_b(self): Galactic latitude calculated from (RA, Dec) """ - return self.galactic.b + return SkyCoord(self).galactic.b @property def ecliptic_lon(self): From b555ac0943152eb06a0a306c904495f4ca5f8a17 Mon Sep 17 00:00:00 2001 From: "Raphael A. P. Oliveira" Date: Thu, 18 Jan 2024 09:02:35 +0100 Subject: [PATCH 6/7] Adapted ecliptic transf to pass test, issue #70 --- source/MulensModel/coordinates.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/source/MulensModel/coordinates.py b/source/MulensModel/coordinates.py index e422448d..accde451 100644 --- a/source/MulensModel/coordinates.py +++ b/source/MulensModel/coordinates.py @@ -40,8 +40,7 @@ def __init__(self, *args, **kwargs): if 'unit' not in kwargs and len(args) > 0: self._check_for_ra_in_degrees(args[0]) kwargs['unit'] = (u.hourangle, u.deg) - frame = kwargs.get('frame') - self._validate_input(args[0], frame) + self._validate_input(args[0], kwargs.get('frame')) SkyCoord.__init__(self, *args, **kwargs) if self.cartesian.xyz.shape not in [(3,), (3, 1)]: @@ -144,7 +143,7 @@ def ecliptic_lon(self): ecliptic longitude calculated from (RA, Dec) """ from astropy.coordinates import GeocentricTrueEcliptic - return self.transform_to(GeocentricTrueEcliptic).lon + return SkyCoord(self).transform_to(GeocentricTrueEcliptic).lon @property def ecliptic_lat(self): @@ -154,7 +153,7 @@ def ecliptic_lat(self): ecliptic latitude calculated from (RA, Dec) """ from astropy.coordinates import GeocentricTrueEcliptic - return self.transform_to(GeocentricTrueEcliptic).lat + return SkyCoord(self).transform_to(GeocentricTrueEcliptic).lat @property def north_projected(self): From 50e9b4ed0242fe0356d252f4b24d77d3915c41ad Mon Sep 17 00:00:00 2001 From: "Raphael A. P. Oliveira" Date: Thu, 18 Jan 2024 10:49:35 +0100 Subject: [PATCH 7/7] Applied pytest to coordinates validation, issue #70 --- source/MulensModel/coordinates.py | 3 +- source/MulensModel/tests/test_Coords.py | 43 +++++++++++++++---------- source/MulensModel/version.py | 2 +- 3 files changed, 28 insertions(+), 20 deletions(-) diff --git a/source/MulensModel/coordinates.py b/source/MulensModel/coordinates.py index accde451..d22ae69e 100644 --- a/source/MulensModel/coordinates.py +++ b/source/MulensModel/coordinates.py @@ -2,6 +2,7 @@ import warnings from astropy.coordinates import SkyCoord, ICRS, FK4, FK5 +from astropy.coordinates import GeocentricTrueEcliptic from astropy import units as u from MulensModel.utils import Utils @@ -142,7 +143,6 @@ def ecliptic_lon(self): ecliptic longitude calculated from (RA, Dec) """ - from astropy.coordinates import GeocentricTrueEcliptic return SkyCoord(self).transform_to(GeocentricTrueEcliptic).lon @property @@ -152,7 +152,6 @@ def ecliptic_lat(self): ecliptic latitude calculated from (RA, Dec) """ - from astropy.coordinates import GeocentricTrueEcliptic return SkyCoord(self).transform_to(GeocentricTrueEcliptic).lat @property diff --git a/source/MulensModel/tests/test_Coords.py b/source/MulensModel/tests/test_Coords.py index a91b2490..87366c9f 100644 --- a/source/MulensModel/tests/test_Coords.py +++ b/source/MulensModel/tests/test_Coords.py @@ -1,6 +1,7 @@ import os import unittest import numpy as np +import pytest from astropy.coordinates import SkyCoord, ICRS, FK4, FK5, Galactic import astropy.units as u @@ -50,23 +51,31 @@ def test_event_coords(): assert event_3.model.coords.to_string('hmsdms') == new_coord_str -def test_coord_validation(): - - c_test = '18h00m00s -30d00m00s' - tests = [mm.Coordinates(c_test), - mm.Coordinates(c_test, frame='fk4'), - mm.Coordinates(SkyCoord(c_test, frame='fk5')), - mm.Coordinates(ICRS(*c_test.split())), - mm.Coordinates(FK4(*c_test.split())), - mm.Coordinates(FK5(*c_test.split()))] - t_errors = [(c_test, 'galactic'), - (SkyCoord(c_test, frame='galactic'), None), - (Galactic(*c_test.split()), None)] - for test in tests: - assert test.to_string('hmsdms') == c_test - for error in t_errors: - with unittest.TestCase().assertRaises(ValueError): - mm.Coordinates(error[0], frame=error[1]) +cval = '18h00m00s -30d00m00s' +tested_cs = [(cval, None), (cval, 'fk4'), + (SkyCoord(cval, frame='fk5'), None), (ICRS(*cval.split()), None), + (FK4(*cval.split()), None), (FK5(*cval.split()), None)] +tested_cs2 = [(cval, 'galactic'), (SkyCoord(cval, frame='galactic'), None), + (Galactic(*cval.split()), None)] + + +@pytest.mark.parametrize("coord_test", tested_cs) +def test_coord_validation(coord_test): + """ + Test Coordinates input to accept frames than icrs, fk4 and fk5. + """ + coord_instance = mm.Coordinates(coord_test[0], frame=coord_test[1]) + assert coord_instance.to_string('hmsdms') == cval + + +@pytest.mark.parametrize("coord_test2", tested_cs2) +def test_coord_validation_error(coord_test2): + """ + Test Coordinates input to reject frames different than icrs, fk4 and fk5. + """ + with unittest.TestCase().assertRaises(ValueError): + mm.Coordinates(coord_test2[0], frame=coord_test2[1]) + def check_event_coords(event, ra, dec): """ diff --git a/source/MulensModel/version.py b/source/MulensModel/version.py index 71fae1bc..2c109d88 100644 --- a/source/MulensModel/version.py +++ b/source/MulensModel/version.py @@ -1 +1 @@ -__version__ = "2.19.3" +__version__ = "2.19.4"