Merge pull request #4 from observatorycontrolsystem/fix/mpc_proper_de…

…signations

Fix/mpc proper designations

.github/workflows/canary_tests.yaml

@@ -0,0 +1,28 @@
+name: CanaryTests
+
+on:
+  # Run this workflow for pushes to main
+  push:
+    branches:
+      - main
+
+jobs:
+  run_tests:
+    runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        python-version: ["3.8", "3.9", "3.10"]
+    steps:
+    - name: Check out repository
+      uses: actions/checkout@v2
+    - name: Set up Python ${{ matrix.python-version }}
+      uses: actions/setup-python@v2
+      with:
+        python-version: ${{ matrix.python-version }}
+    - name: Install dependencies
+      run: |
+        python -m pip install --upgrade pip
+        python -m pip install poetry
+        poetry install
+    - name: Run tests
+      run: poetry run pytest simbad2k/tests/canary_tests_simbad2k.py

simbad2k/simbad2k.py

@@ -5,6 +5,7 @@
 from logging.config import dictConfig
 import math
 import os
+import requests
 
 from astroquery.exceptions import RemoteServiceError
 from flask import Flask, jsonify, request
@@ -82,6 +83,12 @@ def get_result(self):
 
 
 class MPCQuery(object):
+    """
+    Query the Minor Planet Center for orbital elements of a given object.
+    First submit the object's name to the MPC's query-identifier API to get the object's primary designation.
+    Next submit the primary designation to the MPC via astroquery to get the object's orbital elements.
+    Returns a dictionary of the object's orbital elements.
+    """
     def __init__(self, query, scheme):
         self.query = query
         self.keys = [
@@ -93,70 +100,132 @@ def __init__(self, query, scheme):
         self.query_params_mapping = {
             'mpc_minor_planet': ['name', 'designation', 'number'], 'mpc_comet': ['number', 'designation']
         }
+        # Object types as described by the MPC: https://www.minorplanetcenter.net/mpcops/documentation/object-types/
+        # 50 is for Interstellar Objects
+        self.mpc_type_mapping = {'mpc_minor_planet': [0,1,6,20], 'mpc_comet': [6,10,11,20,50]}
         self.scheme = scheme
 
     def _clean_result(self, result):
+        """
+        Converts the results from the MPC into a dictionary of floats.
+        Extracts the object's name from the query results and adds it to the dictionary.
+        """
         cleaned_result = {}
         for key in self.keys:
             try:
                 value = float(result[key])
             except (ValueError, TypeError):
                 value = None
             cleaned_result[key] = value
+        # Build object name from returned Data
+        if result.get('number'):
+            if result.get('name'):
+                # If there is a number and a name, use both with the format "name (number)", otherwise just use number
+                cleaned_result['name'] = f"{result['name']} ({result['number']})"
+            else:
+                cleaned_result['name'] = f"{result['number']}"
+                if result.get('object_type'):
+                    # Add comet object type if it exists
+                    cleaned_result['name'] += result['object_type']
+        else:
+            cleaned_result['name'] = result.get('designation')
         return cleaned_result
 
+    def get_primary_designation(self):
+        """
+        Submit the object's name to the MPC's query-identifier API to get the object's preferred primary and
+        preliminary designations.
+        In the case of multiple possible targets (usually happens for multiple objects with the same name),
+        try to disambiguate with the following criteria:
+            * Choose the first target with a 'permid' that could be converted into an INT if searching for an asteroid.
+            * Return the first target with a 'permid' if searching for a comet.
+            * If no 'permid' is found, query the MPC again using the first target with a preliminary designation.
+        """
+        response = requests.get("https://data.minorplanetcenter.net/api/query-identifier",
+                                data=self.query.replace("+", " ").upper())
+        identifications = response.json()
+        if identifications.get('object_type') and\
+                identifications.get('object_type')[1] not in self.mpc_type_mapping[self.scheme]:
+            return None, None
+        if identifications.get('disambiguation_list'):
+            for target in identifications['disambiguation_list']:
+                if self.scheme_mapping[self.scheme] == 'asteroid':
+                    try:
+                        return int(target['permid']), None
+                    except (ValueError, KeyError):
+                        continue
+                elif self.scheme_mapping[self.scheme] == 'comet':
+                    perm_id = target.get('permid')
+                    if perm_id:
+                        return perm_id, None
+                if target.get('unpacked_primary_provisional_designation'):
+                    # We need to re-check preliminary designations for multiple targets because these are sometimes
+                    # returned by the MPC for disambiguation even though the targets have primary IDs
+                    response = requests.get("https://data.minorplanetcenter.net/api/query-identifier",
+                                data=target['unpacked_primary_provisional_designation'])
+                    identifications = response.json()
+                    break
+        return identifications['permid'], identifications['unpacked_primary_provisional_designation']
+
     def get_result(self):
         from astroquery.mpc import MPC
         schemes = []
         if self.scheme in self.scheme_mapping:
             schemes.append(self.scheme)
         else:
             schemes = [*self.scheme_mapping]
+        # Get the primary designation of the object and preferred provisional designation if available
+        primary_designation, primary_provisional_designation = self.get_primary_designation()
         for scheme in schemes:
-            for query_param in self.query_params_mapping[scheme]:
-                # astroquery has a very robust matching algorythm for "number" and will return elements for 
-                # asteroid #2000 if you ask for "2000 JD1". We do not want to ask for a number when our object name
-                # is actually a preliminary designation. 
-                # Numbered objects will not have a designation.
-                # Comets will have letters in their "numbers" i.e. 12P.
-                if query_param == 'number' and self.scheme_mapping[scheme] == 'asteroid':
+            # Make sure the primary designation can be expressed as an integer for asteroids to keep them from being
+            # confused for comets
+            if primary_designation:
+                if scheme == 'mpc_minor_planet':
                     try:
-                        params = {'target_type': self.scheme_mapping[scheme], query_param: int(self.query)}
+                        primary_designation = int(primary_designation)
                     except ValueError:
                         return None
-                else:
-                    params = {'target_type': self.scheme_mapping[scheme], query_param: self.query}
-                result = MPC.query_objects_async(**params).json()
-                ret_dict = {}
-                if len(result) > 1:
-                    # Return the set of orbital elements closest to the current date
-                    recent = None
-                    recent_time_diff = None
-                    now = datetime.now()
-                    for ephemeris in result:
-                        if not recent or not recent_time_diff:
-                            recent = ephemeris
+                params = {'target_type': self.scheme_mapping[scheme], 'number': primary_designation}
+                designation = primary_designation
+            elif primary_provisional_designation:
+                params = {'target_type': self.scheme_mapping[scheme], 'designation': primary_provisional_designation}
+                designation = primary_provisional_designation
+            else:
+                return None
+            result = MPC.query_objects_async(**params).json()
+            # There are 2 conditions under which we can get back multiple sets of elements:
+            # 1. When the search is for a comet and there are multiple types with the same number (e.g. 1P/1I)
+            # 2. When the search has multiple sets of elements with different epochs
+            if len(result) > 1:
+                # Limit results to those that match the object type
+                results_that_match_query_type = [elements for elements in result
+                                                 if elements.get('object_type', '').lower() in designation.lower()]
+                if results_that_match_query_type:
+                    result = results_that_match_query_type
+            if len(result) > 1:
+                recent = None
+                recent_time_diff = None
+                now = datetime.now()
+                # Select the set of elements that are closest to the current date
+                for elements in result:
+                    if not recent or not recent_time_diff:
+                        recent = elements
+                        recent_time_diff = math.fabs(
+                            (datetime.strptime(recent['epoch'].rstrip('0').rstrip('.'), '%Y-%m-%d') - now).days
+                        )
+                    else:
+                        elements_time_diff = math.fabs(
+                            (datetime.strptime(elements['epoch'].rstrip('0').rstrip('.'), '%Y-%m-%d') - now).days
+                        )
+                        if elements_time_diff < recent_time_diff:
+                            recent = elements
                             recent_time_diff = math.fabs(
                                 (datetime.strptime(recent['epoch'].rstrip('0').rstrip('.'), '%Y-%m-%d') - now).days
                             )
-                        else:
-                            ephemeris_time_diff = math.fabs(
-                                (datetime.strptime(ephemeris['epoch'].rstrip('0').rstrip('.'), '%Y-%m-%d') - now).days
-                            )
-                            if ephemeris_time_diff < recent_time_diff:
-                                recent = ephemeris
-                                recent_time_diff = math.fabs(
-                                    (datetime.strptime(recent['epoch'].rstrip('0').rstrip('.'), '%Y-%m-%d') - now).days
-                                )
-                    ret_dict = self._clean_result(recent)
-                elif len(result) == 1:
-                    ret_dict = self._clean_result(result[0])
-
-                if ret_dict:
-                    ret_dict['name'] = self.query
-                    return ret_dict
-
-            return None
+                return self._clean_result(recent)
+            if result:
+                return self._clean_result(result[0])
+        return None
 
 
 class NEDQuery(object):
@@ -165,17 +234,17 @@ def __init__(self, query, scheme):
         self.scheme = scheme
 
     def get_result(self):
-        from astroquery.ned import Ned
+        from astroquery.ipac.ned import Ned
         ret_dict = {}
         try:
             result_table = Ned.query_object(self.query)
         except RemoteServiceError:
             return None
         if len(result_table) == 0:
             return None
-        ret_dict['ra_d'] = result_table['RA(deg)'][0]
-        ret_dict['dec_d'] = result_table['DEC(deg)'][0]
-        ret_dict['name'] = self.query
+        ret_dict['ra_d'] = result_table['RA'][0]
+        ret_dict['dec_d'] = result_table['DEC'][0]
+        ret_dict['name'] = result_table['Object Name'][0]
         return ret_dict
 
 
@@ -194,6 +263,8 @@ def generate_cache_key(query, scheme, target_type):
 
 @app.route('/<path:query>')
 def root(query):
+    if query == 'favicon.ico':
+        return jsonify({})
     logger.log(msg=f'Received query for target {query}.', level=logging.INFO)
     target_type = request.args.get('target_type', '')
     scheme = request.args.get('scheme', '')
@@ -225,7 +296,7 @@ def index():
                     'For non_sidereal targets, you must include scheme, which can be '
                     'either mpc_minor_planet or mpc_comet.'
                     'Ex: <a href="/103P?target_type=non_sidereal&scheme=mpc_comet">'
-                    '/m51?target_type=sidereal&scheme=mpc_comet</a>')
+                    '/103P?target_type=non_sidereal&scheme=mpc_comet</a>')
     return instructions