Merge pull request #1176 from TOMToolkit/1165-some-targets-are-not-fo…

…und-during-simbad-query

address current limitations in astroquery table join, and update tests

pyproject.toml

@@ -43,7 +43,7 @@ dynamic = ["version"]
 requires-python = ">=3.9.0,<3.13"
 dependencies = [
     "numpy >1.24,<2.2",
-    "astroquery @ git+https://github.com/astropy/astroquery.git@e2db773",
+    "astroquery <1",
     "astroplan >=0.8,<0.11",
     "astropy >=5.3.3,<7 ; python_version >= '3.10'",
     "astropy >=5.3.3,<6 ; python_version < '3.10'",

tom_catalogs/harvesters/simbad.py

@@ -1,7 +1,7 @@
 from tom_catalogs.harvester import AbstractHarvester
 
 from astroquery.simbad import Simbad
-from astroquery.exceptions import TableParseError
+from astropy.table import Table
 
 
 class SimbadHarvester(AbstractHarvester):
@@ -17,17 +17,21 @@ def __init__(self, *args, **kwargs):
         self.simbad.add_votable_fields('pmra', 'pmdec', 'ra', 'dec', 'main_id', 'parallax', 'distance')
 
     def query(self, term):
-        try:
+        self.catalog_data: Table = self.simbad.query_object(term)
+        # astroquery <0.4.10, > 0.4.7 has issues joining the distance field, failing to find any results.
+        # This workaround checks if the query result is an ampty table and then tries the query a 2nd time without the
+        # distance field.
+        if not self.catalog_data:
+            self.simbad.reset_votable_fields()
+            self.simbad.add_votable_fields('pmra', 'pmdec', 'ra', 'dec', 'main_id', 'parallax')
             self.catalog_data = self.simbad.query_object(term)
-        except TableParseError:  # SIMBAD will raise a TableParseError if a result is not found
-            self.catalog_data = None  # The CatalogQueryView will display a proper error if catalog_data is None
 
     def to_target(self):
         target = super().to_target()
         votable_fields = ['RA', 'DEC', 'PMRA', 'PMDEC', 'MAIN_ID', 'MESDISTANCE.dist', 'MESDISTANCE.unit']
         result = {}
         for key in votable_fields:
-            if str(self.catalog_data[key.lower()][0]) not in ['--', '']:
+            if key.lower() in self.catalog_data.colnames and str(self.catalog_data[key.lower()][0]) not in ['--', '']:
                 result[key] = self.catalog_data[key.lower()][0]
         target.type = 'SIDEREAL'
         target.ra = result.get('RA')

tom_catalogs/tests/harvesters/test_simbad.py

@@ -1,6 +1,5 @@
-from unittest.mock import MagicMock
+from unittest.mock import Mock
 
-from astroquery.exceptions import TableParseError
 from astropy.table import Table
 from django.test import tag, TestCase
 
@@ -10,24 +9,37 @@
 class TestSimbadHarvester(TestCase):
     def setUp(self):
         self.broker = SimbadHarvester()
-        table_data = {'ra': [10.68470800], 'dec': [41.26875000],
-                      'pmra': ['--'], 'pmdec': ['--'], 'main_id': ['M 31, 2C 56, DA 21'],
-                      'mesdistance.dist': [0.8200], 'mesdistance.unit': ['Mpc']}
+        table_data = {'main_id': ['M  31'],
+                      'ra': [10.684708333333333],
+                      'dec': [41.268750000000004],
+                      'pmra': ['--'],
+                      'pmdec': ['--'],
+                      'mesdistance.dist': [761.0],
+                      'mesdistance.unit': ['kpc ']}
         self.broker.catalog_data = Table(table_data)
+        self.empty_table_data = {'main_id': [],
+                                 'ra': [],
+                                 'dec': [],
+                                 'pmra': [],
+                                 'pmdec': [],
+                                 'plx_value': [],
+                                 'mesdistance.dist': [],
+                                 'mesdistance.unit': []}
 
     def test_query_failure(self):
-        self.broker.simbad = MagicMock()
-        self.broker.simbad.query_object.side_effect = TableParseError()
+        self.broker.simbad.query_object = Mock(return_value=Table(self.empty_table_data))
         self.broker.query('M31')
-        self.assertIsNone(self.broker.catalog_data)
+        # Check that the empty table was returned and is falsey so it would trigger the MissingDataException in the
+        # AbstractHarvester
+        self.assertFalse(self.broker.catalog_data)
 
     def test_to_target(self):
         target = self.broker.to_target()
         self.assertEqual(target.ra, self.broker.catalog_data['ra'])
         self.assertEqual(target.dec, self.broker.catalog_data['dec'])
         self.assertEqual(target.pm_ra, None)
         self.assertEqual(target.pm_dec, None)
-        self.assertEqual(target.distance, self.broker.catalog_data['mesdistance.dist'] * 1000000)
+        self.assertEqual(target.distance, self.broker.catalog_data['mesdistance.dist'] * 1000)
         self.assertEqual(target.name, 'M31')