Simple PR review fixes.

Author: DinoBektesevic

CHANGES.rst

@@ -4,6 +4,11 @@
 New Tools and Services
 ----------------------
 
+ipac.irsa.most
+^^^^^^^^^^^^^^
+
+- New module to access the Moving Object Search Tool (MOST) added. [#2660]
+
 gaia
 ^^^^
 

astroquery/ipac/irsa/most.py

@@ -22,12 +22,9 @@ class MostClass(BaseQuery):
     URL = conf.most_server
     TIMEOUT = conf.timeout
 
-    def __init__(self):
-        super().__init__()
-
     def _validate_name_input_type(self, params):
         """
-        Validate required parameters when `input_type='name_input'`.
+        Validate required parameters when ``input_type='name_input'``.
 
         Parameters
         ----------
@@ -45,7 +42,7 @@ def _validate_name_input_type(self, params):
 
     def _validate_nafid_input_type(self, params):
         """
-        Validate required parameters when `input_type='naifid_input'`.
+        Validate required parameters when ``input_type='naifid_input'``.
 
 
         Parameters
@@ -64,7 +61,7 @@ def _validate_nafid_input_type(self, params):
 
     def _validate_mpc_input_type(self, params):
         """
-        Validate required parameters when `input_type='mpc_input'`.
+        Validate required parameters when ``input_type='mpc_input'``.
 
         Parameters
         ----------
@@ -88,7 +85,7 @@ def _validate_mpc_input_type(self, params):
 
     def _validate_manual_input_type(self, params):
         """
-        Validate required parameters when `input_type='manual_input'`.
+        Validate required parameters when ``input_type='manual_input'``.
 
         Parameters
         ----------
@@ -103,9 +100,9 @@ def _validate_manual_input_type(self, params):
         """
         obj_type = params.get("obj_type", False)
         if not obj_type:
-            raise ValueError("When input type is 'mpc_input' key 'obj_type' is required.")
+            raise ValueError("When input type is 'manual_input' key 'obj_type' is required.")
         if obj_type not in ("Asteroid", "Comet"):
-            raise ValueError("Object type is case sensitive and must be one of: `Asteroid` or `Comet`")
+            raise ValueError("Object type is case sensitive and must be one of: 'Asteroid' or 'Comet'")
 
         # MOST will always require at least the distance and eccentricity
         # distance param is named differently in cases of asteroids and comets
@@ -128,8 +125,8 @@ def _validate_input(self, params):
         Validate the minimum required set of parameters, for a given input
         type, are at least truthy.
 
-        These include the keys `catalog`, `input_type`, `output_mode` and
-        `ephem_step` in addition to keys required by the specified input type.
+        These include the keys ``catalog``, ``input_type``, ``output_mode`` and
+        ``ephem_step`` in addition to keys required by the specified input type.
 
         Parameters
         ----------
@@ -159,13 +156,13 @@ def _validate_input(self, params):
             self._validate_manual_input_type(params)
         else:
             raise ValueError(
-                "Unrecognized `input_type`. Expected `name_input`, `nafid_input` "
+                "Unrecognized 'input_type'. Expected `name_input`, `nafid_input` "
                 f"`mpc_input` or `manual_input`, got {input_type} instead."
             )
 
     def _parse_full_regular_response(self, response, withTarballs=False):
         """
-        Parses the response when output type is set to `Regular` or `Full`.
+        Parses the response when output type is set to ``"Regular"`` or ``"Full"``.
 
         Parameters
         ----------
@@ -178,14 +175,14 @@ def _parse_full_regular_response(self, response, withTarballs=False):
         Returns
         -------
         retdict : `dict`
-            Dictionary containing the keys `results`, `metadata` and `region`.
-            Optionally can contain keys `fits_tarball` and `region_tarball`.
-            The `results` and `metadata` are an `astropy.table.Table` object
+            Dictionary containing the keys ``results``, ``metadata`` and ``region``.
+            Optionally can contain keys ``fits_tarball`` and ``region_tarball``.
+            The ``results`` and ``metadata`` are an `astropy.table.Table` object
             containing the links to image and region files and minimum object
-            metadata, while `metadata` contains the image metadata and object
-            positions. The `region` key contains a link to the DS9 region file
+            metadata, while ``metadata`` contains the image metadata and object
+            positions. The ``region`` key contains a link to the DS9 region file
             representing the matched object trajectory and search boxes. When
-            existing, `fits_tarball` and `region_tarball` are links to the
+            existing, ``fits_tarball`` and ``region_tarball`` are links to the
             tarball archives of the fits and region images.
         """
         retdict = {}
@@ -220,9 +217,9 @@ def list_catalogs(self):
         catalog_dropdown_options = html.find("select").find_all("option")
 
         catalogs = [tag.string for tag in catalog_dropdown_options]
-        # I think I saw somewhere that some password prompt will pop up for
-        # catalogs listed as '--- Internal use only:' but there are seemingly
-        # no limits to queries there.
+
+        # The Internal-Use-only datasets are free to search in MOST.
+        # The way it is supposed to work is that the images will not be accessible.
         if "--- Internal use only:" in catalogs:
             catalogs.remove("--- Internal use only:")
         return catalogs
@@ -531,8 +528,7 @@ def query_object(self, catalog="wise_merge", input_mode="name_input", output_mod
                                  data=qparams, timeout=self.TIMEOUT)
 
         # service unreachable or some other reason
-        if not response.ok:
-            raise response.raise_for_status()
+        response.raise_for_status()
 
         # MOST will not raise an bad response if the query is bad because they
         # are not a REST API
@@ -550,7 +546,7 @@ def query_object(self, catalog="wise_merge", input_mode="name_input", output_mod
             matches = votable.parse(io.BytesIO(response.content))
             if matches.get_table_by_index(0).nrows == 0:
                 warnings.warn("Number of Matched Image Frames   = 0", NoResultsWarning)
-                return None
+                return Table()
             return matches
         else:
             return self._parse_full_regular_response(response, qparams["fits_region_files"])

astroquery/ipac/irsa/tests/test_most.py

@@ -87,10 +87,10 @@ def patch_requests(request):
 
 def test_validation(patch_requests):
     """Tests MOST parameter validation."""
-    with pytest.raises(ValueError):
+    with pytest.raises(ValueError, match="When input type is 'name_input' key 'obj_name' is required."):
         Most.query_object()
 
-    with pytest.raises(ValueError):
+    with pytest.raises(ValueError, match="When input type is 'name_input' key 'obj_name' is required."):
         Most.query_object(catalog="wise_allsky_4band")
 
     # make sure no funny business happens with
@@ -105,7 +105,7 @@ def test_validation(patch_requests):
         input_mode="name_input"
     )
 
-    with pytest.raises(ValueError):
+    with pytest.raises(ValueError, match="When obj_type is 'Asteroid', 'semimajor_axis' is required."):
         # fails because insufficient orbital parameters specified
         Most.query_object(
             catalog="wise_allsky_4band",
@@ -125,7 +125,7 @@ def test_validation(patch_requests):
         eccentricity=0.33,
     )
 
-    with pytest.raises(ValueError):
+    with pytest.raises(ValueError, match="When obj_type is 'Comet', 'perih_dist' is required."):
         # Comets require perihel_dist keyword
         # instead of smimajor_axis
         Most.query_object(
@@ -137,7 +137,7 @@ def test_validation(patch_requests):
             eccentricity=0.33
         )
 
-    with pytest.raises(ValueError):
+    with pytest.raises(ValueError, match="Object type is case sensitive and must be one of: 'Asteroid' or 'Comet'"):
         # object type is case sensitive
         Most.query_object(
             catalog="wise_allsky_4band",
@@ -148,41 +148,42 @@ def test_validation(patch_requests):
             eccentricity=0.33
         )
 
-    with pytest.raises(ValueError):
+    with pytest.raises(ValueError, match="When input type is 'mpc_input' key 'obj_type' is required."):
         # missing mpc_data and obj_type
         Most.query_object(
             catalog="wise_allsky_4band",
             output_mode="Brief",
             input_mode="mpc_input"
         )
 
-    Most.query_object(
+    response = Most.query_object(
         catalog="wise_allsky_4band",
         output_mode="Brief",
         input_mode="mpc_input",
         obj_type="Asteroid",
         mpc_data="K10N010+2010+08+16.1477+1.494525+0.533798+153.4910+113.2118+12.8762+20100621+17.0+4.0+P/2010+N1+(WISE)+MPC+75712"  # noqa: E501
     )
+    assert len(response) > 0
 
 
 def test_regular(patch_requests):
     """Tests the default MOST query returns expected results."""
-    response = Most.query_object(obj_name="Victoria")
+    results = Most.query_object(obj_name="Victoria")
 
-    assert "results" in response
-    assert "metadata" in response
-    assert "region" in response
-    assert "fits_tarball" not in response
-    assert "region_tarball" not in response
+    assert "results" in results
+    assert "metadata" in results
+    assert "region" in results
+    assert "fits_tarball" not in results
+    assert "region_tarball" not in results
 
-    results = Table.read(data_path("most_regular_results.tbl"), format="ipac")
-    metadata = Table.read(data_path("most_imgframes_matched_final_table.tbl"), format="ipac")
+    expected_results = Table.read(data_path("most_regular_results.tbl"), format="ipac")
+    expected_metadata = Table.read(data_path("most_imgframes_matched_final_table.tbl"), format="ipac")
     url = REGULAR_BASE_URL + "ds9region/ds9_orbit_path.reg"
 
     silent_stream = io.StringIO()
-    assert report_diff_values(results, response["results"], silent_stream)
-    assert report_diff_values(metadata, response["metadata"], silent_stream)
-    assert url == response["region"]
+    assert report_diff_values(expected_results, results["results"], silent_stream)
+    assert report_diff_values(expected_metadata, results["metadata"], silent_stream)
+    assert url == results["region"]
 
 
 def test_get_full_with_tarballs(patch_requests):

astroquery/ipac/irsa/tests/test_most_remote.py

@@ -1,14 +1,13 @@
 # Licensed under a 3-clause BSD style license - see LICENSE.rst
 import os
 import pytest
-import warnings
 
 import numpy as np
 
 import astropy.io.fits as fits
 from astropy.table import Table
 from astroquery.ipac.irsa import Most
-from astroquery.exceptions import InvalidQueryError
+from astroquery.exceptions import InvalidQueryError, NoResultsWarning
 
 # each MOST query is given a PID and a temporary directory availible publicly
 # from this base URL. This URL is then used to create links on the returned
@@ -26,37 +25,38 @@ def data_path(filename):
 @pytest.mark.remote_data
 def test_query_object():
     """Tests the default MOST query returns expected results."""
-    response = Most.query_object(
+    results = Most.query_object(
         obj_name="Victoria",
         obs_begin="2014-05-21",
         obs_end="2014-05-30"
     )
 
     # check expected fields were returned
-    assert "results" in response
-    assert "metadata" in response
-    assert "region" in response
-    assert "fits_tarball" not in response
-    assert "region_tarball" not in response
+    assert "results" in results
+    assert "metadata" in results
+    assert "region" in results
+    assert "fits_tarball" not in results
+    assert "region_tarball" not in results
 
     # check content is as expected for the columns we don't expect to change
-    # This excludes URLS, they contain PIDs. The rest can be compared, but the
+    # This excludes URLS; they contain PIDs. The rest can be compared, but the
     # problem is that report_diff_values doesn't sort the values and they are
     # not returned in same order nor header width. So we have to compare
     # manually
-    results = Table.read(data_path("most_regular_results.tbl"), format="ipac")
+    expected_results = Table.read(data_path("most_regular_results.tbl"), format="ipac")
 
-    columns = [col for col in results.columns if col not in ("image_url", "postcard_url", "region_file")]
+    columns = [col for col in expected_results.columns if col not in ("image_url", "postcard_url", "region_file")]
     for col in columns:
-        response_col = response["results"][col]
-        results_col = results[col]
-        response_col.sort()
+        results_col = results["results"][col]
+        expected_results_col = expected_results[col]
         results_col.sort()
-        if 'float' in response_col.dtype.name:
-            assert np.allclose(response_col, results_col, rtol=0.005)
+        expected_results_col.sort()
+        if 'float' in results_col.dtype.name:
+            assert np.allclose(results_col, expected_results_col, rtol=0.005)
         else:
-            # probably a string like Img_ID or datetime stamps
-            assert all(response_col == results_col)
+            # probably a string like Img_ID or datetime stamps so direct
+            # comparison is ok
+            assert all(results_col == expected_results_col)
 
 
 @pytest.mark.remote_data
@@ -91,8 +91,7 @@ def test_list_catalogs():
 
 @pytest.mark.remote_data
 def test_no_results():
-    with warnings.catch_warnings():
-        warnings.simplefilter("ignore")
+    with pytest.warns(NoResultsWarning, match="Number of Matched Image Frames   = 0"):
         response = Most.query_object(
             obj_name="Victoria",
             obs_begin="2019-05-21",
@@ -101,8 +100,7 @@ def test_no_results():
 
     assert response is None
 
-    with warnings.catch_warnings():
-        warnings.simplefilter("ignore")
+    with pytest.warns(NoResultsWarning, match="Number of Matched Image Frames   = 0"):
         response = Most.get_images(
             obj_name="Victoria",
             obs_begin="2019-05-21",
@@ -114,14 +112,16 @@ def test_no_results():
 
 @pytest.mark.remote_data
 def test_invalid_query():
-    with pytest.raises(InvalidQueryError):
+    err_msg = r".*observation time start time must be less than end time.*"
+
+    with pytest.raises(InvalidQueryError, match=err_msg):
         Most.query_object(
             obj_name="Victoria",
             obs_begin="2014-05-21",
             obs_end="2014-05-19"
         )
 
-    with pytest.raises(InvalidQueryError):
+    with pytest.raises(InvalidQueryError, match=err_msg):
         Most.get_images(
             obj_name="Victoria",
             obs_begin="2014-05-21",

docs/ipac/irsa/most/most.rst

@@ -97,7 +97,7 @@ dist_ctr       Projected distance  from the center of the image (deg).
 phase          Sun-Object-earth angle (deg).
 vmag           Estimate of visual magnitude (from Horizons).
 image_url      Links to download or view the data.
-postcard_url   ??? usually ``null``
+postcard_url   Currently ``null``.  It is waiting for a future dataset that may have a png along with the image.
 region_file    Markers for the moving object in DS9 "region" format.
 ============   ===============================================================
 
@@ -445,5 +445,4 @@ key:
 Reference/API
 =============
 
-.. automodule:: astroquery.ipac.irsa.most
-   :members:
+See `~astroquery.ipac.irsa.most.MostClass` and `~astroquery.ipac.irsa.Conf`.