Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better migration simbad #3186

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
-------------------------------------------------------
Expand Down
47 changes: 28 additions & 19 deletions astroquery/simbad/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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")))
Expand All @@ -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))
Expand Down
44 changes: 42 additions & 2 deletions astroquery/simbad/data/query_criteria_fields.json
Original file line number Diff line number Diff line change
Expand Up @@ -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_",
Expand Down Expand Up @@ -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_",
Expand All @@ -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",
Expand All @@ -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"
Expand Down Expand Up @@ -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",
Expand Down
38 changes: 35 additions & 3 deletions astroquery/simbad/tests/test_simbad.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.*"):
Expand All @@ -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):
Expand Down
18 changes: 17 additions & 1 deletion astroquery/simbad/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions docs/simbad/simbad.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
---------------------

Expand Down
Loading