Merge pull request #3186 from cds-astro/better-migration-simbad

bsipocz · web-flow · commit 1944001a0031 · 2025-01-22T14:47:16.000-08:00
Better migration simbad
diff --git a/CHANGES.rst b/CHANGES.rst
@@ -19,6 +19,18 @@ jplspec
 
 - minor improvement to lookuptable behavior [#3173,#2901]
 
+simbad
+^^^^^^
+
+- Fixed adding a list of fluxes with the deprecated notation 
+  ``Simbad.add_votable_fields("flux(U)", "flux(J)")`` [#3186]
+
+- Support more of the 0.4.7 votable fields. Raise more significant error messages
+  for the discontinued ones. [#3186]
+
+- Fix the deprecated votable fields ``otype(V)`` and ``otype(S)`` [#3186]
+
+- Fixed non existing flux filters as votable fields would fail silently [#3186]
 
 Infrastructure, Utility and Other Changes and Additions
 -------------------------------------------------------
diff --git a/astroquery/simbad/core.py b/astroquery/simbad/core.py
@@ -234,18 +234,19 @@ def list_votable_fields(self):
         >>> options = Simbad.list_votable_fields() # doctest: +REMOTE_DATA
         >>> # to print only the available bundles of columns
         >>> options[options["type"] == "bundle of basic columns"][["name", "description"]] # doctest: +REMOTE_DATA
-        <Table length=8>
-            name                         description
-            object                           object
-        ------------- ----------------------------------------------------
-          coordinates                  all fields related with coordinates
-                  dim          major and minor axis, angle and inclination
-           dimensions              all fields related to object dimensions
-            morphtype         all fields related to the morphological type
-             parallax                     all fields related to parallaxes
-        propermotions           all fields related with the proper motions
-                   sp            all fields related with the spectral type
-             velocity all fields related with radial velocity and redshift
+        <Table length=9>
+             name                           description
+            object                             object
+        ------------- -------------------------------------------------------
+          coordinates                     all fields related with coordinates
+                  dim             major and minor axis, angle and inclination
+           dimensions                 all fields related to object dimensions
+            morphtype            all fields related to the morphological type
+             parallax                        all fields related to parallaxes
+                   pm proper motion values in right ascension and declination
+        propermotions              all fields related with the proper motions
+                   sp               all fields related with the spectral type
+             velocity    all fields related with radial velocity and redshift
         """
         # get the tables with a simple link to basic
         query_tables = """SELECT DISTINCT table_name AS name, tables.description
@@ -380,29 +381,33 @@ def add_votable_fields(self, *args):
         """
 
         # the legacy way of adding fluxes
-        args = list(args)
+        args = set(args)
         fluxes_to_add = []
-        for arg in args:
+        args_copy = args.copy()
+        for arg in args_copy:
             if arg.startswith("flux_"):
                 raise ValueError("The votable fields 'flux_***(filtername)' are removed and replaced "
                                  "by 'flux' that will add all information for every filters. "
                                  "See section on filters in "
                                  "https://astroquery.readthedocs.io/en/latest/simbad/simbad_evolution.html"
                                  " to see the new ways to interact with SIMBAD's fluxes.")
             if re.match(r"^flux.*\(.+\)$", arg):
-                warnings.warn("The notation 'flux(U)' is deprecated since 0.4.8 in favor of 'U'. "
-                              "See section on filters in "
+                filter_name = re.findall(r"\((\w+)\)", arg)[0]
+                warnings.warn(f"The notation 'flux({filter_name})' is deprecated since 0.4.8 in favor of "
+                              f"'{filter_name}'. You will see the column appearing with its new name "
+                              "in the output. See section on filters in "
                               "https://astroquery.readthedocs.io/en/latest/simbad/simbad_evolution.html "
                               "to see the new ways to interact with SIMBAD's fluxes.", DeprecationWarning, stacklevel=2)
-                fluxes_to_add.append(re.findall(r"\((\w+)\)", arg)[0])
+                fluxes_to_add.append(filter_name)
                 args.remove(arg)
 
         # output options
         output_options = self.list_votable_fields()
         # fluxes are case-dependant
-        fluxes = output_options[output_options["type"] == "filter name"]["name"]
+        fluxes = set(output_options[output_options["type"] == "filter name"]["name"])
         # add fluxes
-        fluxes_to_add += [flux for flux in args if flux in fluxes]
+        fluxes_from_names = set(flux for flux in args if flux in fluxes)
+        fluxes_to_add += fluxes_from_names
         if fluxes_to_add:
             self.joins.append(_Join("allfluxes", _Column("basic", "oid"),
                               _Column("allfluxes", "oidref")))
@@ -413,6 +418,10 @@ def add_votable_fields(self, *args):
                     self.columns_in_output.append(_Column("allfluxes", flux + "_", flux))
                 else:
                     self.columns_in_output.append(_Column("allfluxes", flux))
+        # remove the arguments already added
+        args -= fluxes_from_names
+        # remove filters from output options
+        output_options = output_options[output_options["type"] != "filter name"]
 
         # casefold args because we allow case difference for every other argument (legacy behavior)
         args = set(map(str.casefold, args))
diff --git a/astroquery/simbad/data/query_criteria_fields.json b/astroquery/simbad/data/query_criteria_fields.json
@@ -33,6 +33,20 @@
     "tap_column": ["ra", "dec", "ra_prec", "dec_prec"],
     "type": "bundle"
   },
+  "dec(d)": {
+    "tap_column": "dec",
+    "type": "alias"
+  },
+  "dec_prec": {
+    "description": "precision code for the coordinates (ra and dec are no longer distinct)",
+    "tap_column": "coo_qual",
+    "type": "alias"
+  },
+  "diameter": {
+    "descriptions": "all measurements of diameter",
+    "tap_table": "mesdiameter",
+    "type": "alias table"
+  },
   "dim": {
     "description": "major and minor axis, angle and inclination",
     "tap_startswith": "galdim_",
@@ -187,14 +201,22 @@
     "tap_column": "morph_type",
     "type": "alias"
   },
-  "otype(V)": {
+  "otype(v)": {
     "tap_table": "otypedef",
     "type": "alias table"
   },
-  "otype(S)": {
+  "otype(n)": {
     "tap_table": "otypedef",
     "type": "alias table"
   },
+  "otype(3)": {
+    "tap_column": "otype",
+    "type": "alias"
+  },
+  "otype(s)": {
+    "tap_column": "otype",
+    "type": "alias"
+  },
   "parallax": {
     "description": "all fields related to parallaxes",
     "tap_startswith": "plx_",
@@ -210,6 +232,11 @@
     "tap_column": "plx_err",
     "type": "alias"
   },
+  "pm": {
+    "description": "proper motion values in right ascension and declination",
+    "tap_column": ["pmra", "pmdec"],
+    "type": "bundle"
+  },
   "pm_err_maja": {
     "description": "major axis of the error ellipse",
     "tap_column": "pm_err_maj",
@@ -225,11 +252,20 @@
     "tap_startswith": "pm",
     "type": "bundle"
   },
+  "ra(d)": {
+    "tap_column": "ra",
+    "type": "alias"
+  },
   "radvel": {
     "description": "Radial velocity",
     "tap_column": "rvz_radvel",
     "type": "alias"
   },
+  "ra_prec": {
+    "description": "precision code for the coordinates (ra and dec are no longer distinct)",
+    "tap_column": "coo_qual",
+    "type": "alias"
+  },
   "redshift": {
     "type": "alias",
     "tap_column": "rvz_redshift"
@@ -268,6 +304,10 @@
     "type": "alias",
     "tap_column": "sp_type"
   },
+  "td1": {
+    "description": "UV fluxes from TD1 satellite,by Thompson et al.",
+    "type": "historical measurement"
+  },
   "v*": {
     "description": "variable stars parameters extracted mainly from the General Catalog of Variable Stars by Kukarkin et al. USSR Academy of Sciences (3rd edition in 1969,and continuations)",
     "type": "alias table",
diff --git a/astroquery/simbad/tests/test_simbad.py b/astroquery/simbad/tests/test_simbad.py
@@ -261,15 +261,26 @@ def test_add_votable_fields():
     # a table which name has changed should raise a warning too
     with pytest.warns(DeprecationWarning, match="'distance' has been renamed 'mesdistance'*"):
         simbad_instance.add_votable_fields("distance")
+
+
+@pytest.mark.usefixtures("_mock_simbad_class")
+@pytest.mark.usefixtures("_mock_basic_columns")
+@pytest.mark.usefixtures("_mock_linked_to_basic")
+def test_add_votable_fields_errors():
     # errors are raised for the deprecated fields with options
     simbad_instance = simbad.SimbadClass()
     with pytest.raises(ValueError, match=r"The votable fields \'flux_\*\*\*\(filtername\)\' are removed *"):
         simbad_instance.add_votable_fields("flux_error(u)")
-    with pytest.warns(DeprecationWarning, match=r"The notation \'flux\(U\)\' is deprecated since 0.4.8 *"):
+    with pytest.warns(DeprecationWarning, match=r"The notation \'flux\(u\)\' is deprecated since 0.4.8 *"):
         simbad_instance.add_votable_fields("flux(u)")
         assert "u_" in str(simbad_instance.columns_in_output)
+    # big letter J filter exists, but not small letter j
+    with pytest.raises(ValueError, match="'j' is not one of the accepted options *"):
+        simbad_instance.add_votable_fields("j")
     with pytest.raises(ValueError, match="Coordinates conversion and formatting is no longer supported*"):
-        simbad_instance.add_votable_fields("coo(s)", "dec(d)")
+        simbad_instance.add_votable_fields("coo(s)")
+    with pytest.warns(DeprecationWarning, match=r"\'dec\(d\)\' has been renamed \'dec\'. *"):
+        simbad_instance.add_votable_fields("dec(d)")
     with pytest.raises(ValueError, match="Catalog Ids are no longer supported as an output option.*"):
         simbad_instance.add_votable_fields("ID(Gaia)")
     with pytest.raises(ValueError, match="Selecting a range of years for bibcode is removed.*"):
@@ -281,7 +292,28 @@ def test_add_votable_fields():
     with pytest.raises(ValueError, match="'alltype' is not one of the accepted options which can be "
                        "listed with 'list_votable_fields'. Did you mean 'alltypes' or 'otype' or 'otypes'?"):
         simbad_instance.add_votable_fields("ALLTYPE")
-    # bundles and tables require a connection to the tap_schema and are thus tested in test_simbad_remote
+    # successive positions no longer ins SIMBAD (for years)
+    with pytest.raises(ValueError, match="Successive measurements of the positions *"):
+        simbad_instance.add_votable_fields("pos")
+    # no longer stores sp_nature
+    with pytest.raises(ValueError, match="Spectral nature is no longer stored in SIMBAD. *"):
+        simbad_instance.add_votable_fields("sp_nature")
+    # typed_id had only been added for astroquery's interaction with the old API
+    with pytest.raises(ValueError, match="'typed_id' is no longer a votable field. *"):
+        simbad_instance.add_votable_fields("typed_id")
+    # uvb and others no longer have their table in SIMBAD
+    with pytest.raises(ValueError, match="Magnitudes are now handled very differently in SIMBAD. *"):
+        simbad_instance.add_votable_fields("ubv")
+
+
+@pytest.mark.usefixtures("_mock_simbad_class")
+@pytest.mark.usefixtures("_mock_basic_columns")
+@pytest.mark.usefixtures("_mock_linked_to_basic")
+def test_add_list_of_fluxes():
+    # regression test for https://github.com/astropy/astroquery/issues/3185#issuecomment-2599191953
+    simbad_instance = simbad.Simbad()
+    with pytest.warns(DeprecationWarning, match=r"The notation \'flux\([UJ]\)\' is deprecated since 0.4.8 *"):
+        simbad_instance.add_votable_fields("flux(U)", "flux(J)")
 
 
 def test_list_wildcards(capsys):
diff --git a/astroquery/simbad/utils.py b/astroquery/simbad/utils.py
@@ -33,13 +33,29 @@ def _catch_deprecated_fields_with_arguments(votable_field):
                          "Coordinates are now per default in degrees and in the ICRS frame.")
     if votable_field.startswith("id("):
         raise ValueError("Catalog Ids are no longer supported as an output option. "
-                         "A good replacement can be `~astroquery.simbad.SimbadClass.query_cat`")
+                         "Good replacements can be `~astroquery.simbad.SimbadClass.query_cat` "
+                         "or `~astroquery.simbad.SimbadClass.query_objectids`.")
     if votable_field.startswith("bibcodelist("):
         raise ValueError("Selecting a range of years for bibcode is removed. You can still use "
                          "bibcodelist without parenthesis and get the full list of bibliographic references.")
     if votable_field in ["membership", "link_bibcode"]:
         raise ValueError("The hierarchy information is no longer an additional field. "
                          "It has been replaced by the 'query_hierarchy' method.")
+    if votable_field in ["pos", "posa"]:
+        raise ValueError("Successive measurements of the positions are no longer stored "
+                         "in SIMBAD. The columns 'ra' and 'dec' contain the most precise "
+                         "measurement recorded by the SIMBAD team. For historical values, "
+                         "search within VizieR (accessible via 'astroquery.vizier').")
+    if votable_field == "sp_nature":
+        raise ValueError("Spectral nature is no longer stored in SIMBAD. You can get the "
+                         "of the spectral type classification in 'sp_bibcode'.")
+    if votable_field == "typed_id":
+        raise ValueError("'typed_id' is no longer a votable field. It is now added by "
+                         "default in 'query_objects' and 'query_region'")
+    if votable_field in ["ubv", "uvby1", "uvby"]:
+        raise ValueError("Magnitudes are now handled very differently in SIMBAD. See this "
+                         "section of the documentation: "
+                         "https://astroquery.readthedocs.io/en/latest/simbad/simbad_evolution.html#optical-filters")
 
 # ----------------------------
 # Support wildcard argument
diff --git a/docs/simbad/simbad.rst b/docs/simbad/simbad.rst
@@ -383,6 +383,9 @@ For example to get the 10 biggest catalogs in SIMBAD, it looks like this:
 
 Where you can remove ``TOP 10`` to get **all** the catalogues (there's a lot of them).
 
+.. warning::
+    This method is case-sensitive since version 0.4.8 
+
 Bibliographic queries
 ---------------------
 
