From 5bb65a8d82ff7c1b3ffe2b781e64fcc8e8ca88dc Mon Sep 17 00:00:00 2001
From: Jan David Mol <mol@astron.nl>
Date: Wed, 2 Mar 2022 08:33:08 +0100
Subject: [PATCH] L2SS-554: Test is_valid_direction, make its implementation
 more solid, and use it when converting directions

---
 .../tangostationcontrol/beam/delays.py        | 11 +++++----
 .../test/beam/test_delays.py                  | 23 +++++++++++++++++++
 2 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/tangostationcontrol/tangostationcontrol/beam/delays.py b/tangostationcontrol/tangostationcontrol/beam/delays.py
index ff81bd485..59d566a1e 100644
--- a/tangostationcontrol/tangostationcontrol/beam/delays.py
+++ b/tangostationcontrol/tangostationcontrol/beam/delays.py
@@ -53,18 +53,19 @@ class delay_calculator:
     def is_valid_direction(self, direction):
         try:
             _ = self.measure.direction(*direction)
-        except RuntimeError as e:
+        except (RuntimeError, TypeError) as e:
             return False
 
         return True
 
     def convert(self, direction, antenna_itrf: list([float])):
-        try:
-            # obtain the direction vector for a specific pointing
-            pointing = self.measure.direction(*direction)
-        except RuntimeError as e:
+        # explicitly check validity to get a proper error message
+        if not self.is_valid_direction(direction):
             raise ValueError(f"Could not convert direction {direction} into a pointing") from e
 
+        # obtain the direction vector for a specific pointing
+        pointing = self.measure.direction(*direction)
+
         reference_dir_vector = self.get_direction_vector(pointing)
 
         # # compute the delays for an antennas w.r.t. the reference position
diff --git a/tangostationcontrol/tangostationcontrol/test/beam/test_delays.py b/tangostationcontrol/tangostationcontrol/test/beam/test_delays.py
index 82b4c2b9a..3b3900138 100644
--- a/tangostationcontrol/tangostationcontrol/test/beam/test_delays.py
+++ b/tangostationcontrol/tangostationcontrol/test/beam/test_delays.py
@@ -18,6 +18,29 @@ class TestDelays(base.TestCase):
 
         self.assertIsNotNone(d)
 
+    def test_is_valid_direction(self):
+        d = delay_calculator([0, 0, 0])
+
+        # should accept base use cases
+        self.assertTrue(d.is_valid_direction(("J2000", "0deg", "0deg")))
+        self.assertTrue(d.is_valid_direction(("J2000", "270deg", "90deg")))
+        self.assertTrue(d.is_valid_direction(("AZELGEO", "0deg", "0deg")))
+        self.assertTrue(d.is_valid_direction(("AZELGEO", "270deg", "90deg")))
+        self.assertTrue(d.is_valid_direction(("SUN", "0deg", "0deg")))
+
+        # i dont get these either, but casacore accepts them
+        self.assertTrue(d.is_valid_direction([]))
+        self.assertTrue(d.is_valid_direction(("J2000",)))
+        self.assertTrue(d.is_valid_direction(("J2000", "0deg",)))
+        self.assertTrue(d.is_valid_direction(("J2000", "0deg", "0deg", "0deg")))
+
+        # should not throw, and return False, on bad uses
+        self.assertFalse(d.is_valid_direction(("", "", "")))
+        self.assertFalse(d.is_valid_direction(("J2000", "0deg", "0deg", "0deg", "0deg")))
+        self.assertFalse(d.is_valid_direction((1, 2, 3)))
+        self.assertFalse(d.is_valid_direction("foo"))
+        self.assertFalse(d.is_valid_direction(None))
+
     def test_sun(self):
         # # create a frame tied to the reference position
         reference_itrf = [3826577.066, 461022.948, 5064892.786]
-- 
GitLab