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

Deprecate raw response from non-async method #3089

Merged
merged 16 commits into from
Sep 6, 2024
Merged
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
23 changes: 22 additions & 1 deletion CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,23 @@ mpc
^^^

- Parse star catalog information when querying observations database [#2957]

- Parse ephemeris with sky motion with three digit precision [#3026]
- Raise EmptyResponseError when empty ephemeris reponse is returned [#3026]

- Raise EmptyResponseError when empty ephemeris response is returned [#3026]

- Deprecate ``get_raw_response`` parameter from ``MPC.get_observations``. The
raw response may be retrieved from the _async() method. [#3089]

- Remove ``get_raw_response`` parameter from ``MPC.get_ephemeris`` and
``MPC.get_observatory_codes`` without deprecation as the parameters were
ignored and had no effect. [#3089]

- Fix bug in ``MPC.get_ephemeris`` that caused the ``cache`` keyword parameter
to be ignored. [#3089]

- Remove ``comettype`` parameter from ``MPC.get_observations`` without
deprecation: it was undocumented, ignored, and had no effect. [#3089]

linelists.cdms
^^^^^^^^^^^^^^
Expand Down Expand Up @@ -191,6 +206,12 @@ mast
- Support for case-insensitive criteria keyword arguments in ``mast.Observations.query_criteria`` and
``mast.Catalogs.query_criteria``. [#3087]

mpc
^^^

- Rename ``MPC.get_mpc_object_endpoint`` to ``MPC._get_mpc_object_endpoint`` to
indicate that it is a private method. [#3089]


0.4.7 (2024-03-08)
==================
Expand Down
31 changes: 15 additions & 16 deletions astroquery/mpc/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
from . import conf
from ..utils import async_to_sync, class_or_instance
from ..exceptions import InvalidQueryError, EmptyResponseError
from astropy.utils.decorators import deprecated_renamed_argument

__all__ = ['MPCClass']

Expand Down Expand Up @@ -186,7 +187,7 @@ def query_object_async(self, target_type, *, get_query_payload=False, **kwargs):

"""

self.get_mpc_object_endpoint(target_type)
self._get_mpc_object_endpoint(target_type)

kwargs['limit'] = 1
return self.query_objects_async(target_type, get_query_payload=get_query_payload, **kwargs)
Expand Down Expand Up @@ -315,7 +316,7 @@ def query_objects_async(self, target_type, *, get_query_payload=False, **kwargs)
Limit the number of results to the given value

"""
mpc_endpoint = self.get_mpc_object_endpoint(target_type)
mpc_endpoint = self._get_mpc_object_endpoint(target_type)

if (target_type == 'comet'):
kwargs['order_by_desc'] = "epoch"
Expand All @@ -329,7 +330,7 @@ def query_objects_async(self, target_type, *, get_query_payload=False, **kwargs)
auth = (self.MPC_USERNAME, self.MPC_PASSWORD)
return self._request('GET', mpc_endpoint, params=request_args, auth=auth)

def get_mpc_object_endpoint(self, target_type):
def _get_mpc_object_endpoint(self, target_type):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is technically an API change (though wasn't used in the docs, nor has any docstrings in the API docs), so I would say, mentioning the cleanup the changelog would be sufficient.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

mpc_endpoint = self.MPC_URL
if target_type == 'asteroid':
mpc_endpoint = mpc_endpoint + '/search_orbits'
Expand All @@ -344,8 +345,7 @@ def get_ephemeris_async(self, target, *, location='500', start=None, step='1d',
proper_motion='total', proper_motion_unit='arcsec/h',
suppress_daytime=False, suppress_set=False,
perturbed=True, unc_links=False,
get_query_payload=False,
get_raw_response=False, cache=False):
get_query_payload=False, cache=False):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changelog says deprecate, yet outright removed. Consider adding the decorator for now.

Copy link
Contributor Author

@mkelley mkelley Sep 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one was removed rather than deprecated because the parameter was unused and had no effect on the method's behavior. I can deprecate it even though it doesn't do anything, or just add a comment in the change log.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same comment for get_observatory_codes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mentioned these changes (and justification) in the change log.

r"""
Object ephemerides from the Minor Planet Ephemeris Service.

Expand Down Expand Up @@ -436,10 +436,6 @@ def get_ephemeris_async(self, target, *, location='500', start=None, step='1d',
Return the HTTP request parameters as a dictionary
(default: ``False``).

get_raw_response : bool, optional
Return raw data without parsing into a table (default:
``False``).

cache : bool
Defaults to False. If set overrides global caching behavior.
See :ref:`caching documentation <astroquery_cache>`.
Expand Down Expand Up @@ -486,7 +482,7 @@ def get_ephemeris_async(self, target, *, location='500', start=None, step='1d',
| P/2003 CP7 | Comet P/2003 CP7 (LINEAR-NEAT) |
+------------+-----------------------------------+

For comets, P/ and C/ are interchangable. The designation
For comets, P/ and C/ are interchangeable. The designation
may also be in a packed format:

+------------+-----------------------------------+
Expand Down Expand Up @@ -601,21 +597,18 @@ def get_ephemeris_async(self, target, *, location='500', start=None, step='1d',
return request_args

self.query_type = 'ephemeris'
response = self._request('POST', self.MPES_URL, data=request_args)
response = self._request('POST', self.MPES_URL, data=request_args, cache=cache)

return response

@class_or_instance
def get_observatory_codes_async(self, *, get_raw_response=False, cache=True):
def get_observatory_codes_async(self, *, cache=True):
"""
Table of observatory codes from the IAU Minor Planet Center.


Parameters
----------
get_raw_response : bool, optional
Return raw data without parsing into a table (default:
`False`).

cache : bool
Defaults to True. If set overrides global caching behavior.
Expand Down Expand Up @@ -771,9 +764,10 @@ def _args_to_ephemeris_payload(self, **kwargs):
return request_args

@class_or_instance
@deprecated_renamed_argument("get_raw_response", None, since="0.4.8",
alternative="async methods")
def get_observations_async(self, targetid, *,
id_type=None,
comettype=None,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what happened with this parameter?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was an unused parameter and doesn't correspond to anything in the API. I can make a comment in the change log.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mentioned in the change log.

get_mpcformat=False,
get_raw_response=False,
get_query_payload=False,
Expand All @@ -783,6 +777,11 @@ def get_observations_async(self, targetid, *,
from the `Minor Planet Center observations database
<https://minorplanetcenter.net/db_search>`_.

.. deprecated:: 0.4.8
The ``get_raw_response`` keyword argument is deprecated. The
`~MPCClass.get_observations_async` method will return a raw response.


Parameters
----------
targetid : int or str
Expand Down
61 changes: 0 additions & 61 deletions astroquery/mpc/tests/data/1994XG_ephemeris_500-a-t.html

This file was deleted.

Loading
Loading