Merge pull request #205 from sot/include-refactor

Refactor process_include_ids to a common base class

proseco/acq.py

@@ -421,16 +421,7 @@ def process_include_ids(self, cand_acqs, stars):
                              kind='nearest', fill_value='extrapolate')
         self.include_halfws = grid_func(self.include_halfws).tolist()
 
-        for include_id in self.include_ids:
-            if include_id not in cand_acqs['id']:
-                try:
-                    star = stars.get_id(include_id)
-                except KeyError:
-                    raise ValueError(f'cannot include star id={include_id} that is not '
-                                     f'a valid star in the ACA field of view')
-                else:
-                    cand_acqs.add_row(star)
-                    self.log(f'Included star id={include_id} in cand_acqs')
+        super().process_include_ids(cand_acqs, stars)
 
     def select_best_p_acqs(self, cand_acqs, min_p_acq, acq_indices, box_sizes):
         """

proseco/core.py

@@ -590,6 +590,26 @@ def guides(self):
     def guides(self, val):
         self.meta['guides'] = val
 
+    def process_include_ids(self, cand_stars, stars):
+        """Ensure that the candidate acqs/guides table has stars that were forced to be included.
+
+        Updates ``cand_stars`` in place.
+
+        :param cand_stars: candidate acquisition or guide stars table
+        :param stars: stars table
+
+        """
+        for include_id in self.include_ids:
+            if include_id not in cand_stars['id']:
+                try:
+                    star = stars.get_id(include_id)
+                except KeyError:
+                    raise ValueError(f'cannot include star id={include_id} that is not '
+                                     f'a valid star in the ACA field of view')
+                else:
+                    cand_stars.add_row(star)
+                    self.log(f'Included star id={include_id} in candidates table')
+
     def log(self, data, **kwargs):
         """
         Add a log event to self.log_info['events'] include ``data`` (which

proseco/guide.py

@@ -263,26 +263,17 @@ def search_stage(self, stage):
     def process_include_ids(self, cand_guides, stars):
         """Ensure that the cand_guides table has stars that were forced to be included.
 
-        Also do validation of include_ids
-
         :param cand_guides: candidate guide stars table
         :param stars: stars table
 
         """
-        for include_id in self.include_ids:
-            if include_id not in cand_guides['id']:
-                try:
-                    star = stars.get_id(include_id)
-                    if ((star['CLASS'] != 0) |
-                        (np.abs(star['row']) >= CHAR.max_ccd_row) |
-                        (np.abs(star['col']) >= CHAR.max_ccd_col)):
-                        raise ValueError("Not a valid candidate")
-                except (ValueError, KeyError):
-                    raise ValueError(f'cannot include star id={include_id} that is not '
-                                     f'a valid star in the ACA field of view')
-                else:
-                    cand_guides.add_row(star)
-                    self.log(f'Included star id={include_id} put in cand_guides')
+        row_max = CCD['row_max'] - CCD['row_pad'] - CCD['window_pad']
+        col_max = CCD['col_max'] - CCD['col_pad'] - CCD['window_pad']
+
+        ok = ((np.abs(stars['row']) < row_max) &
+              (np.abs(stars['col']) < col_max))
+
+        super().process_include_ids(cand_guides, stars[ok])
 
     def get_initial_guide_candidates(self):
         """

proseco/tests/test_guide.py

@@ -320,6 +320,42 @@ def test_guides_include_exclude():
 
 dither_cases = [(8, 8), (64, 8), (8, 64), (20, 20), (30, 20)]
 
+def test_guides_include_bad():
+    """
+    Test include stars for guide where star is bad for some reason.
+
+    - Including a class=1 star on the CCD is allowed.
+    - Including a star (otherwise acceptable) just off the CCD is not allowed.
+
+    """
+    row_max = CCD['row_max'] - CCD['row_pad'] - CCD['window_pad']
+    col_max = CCD['col_max'] - CCD['col_pad'] - CCD['window_pad']
+
+    stars = StarsTable.empty()
+
+    stars.add_fake_constellation(mag=[7.0, 7.1, 7.2, 7.3, 7.4, 7.5, 7.6, 7.7],
+                                 id=[1, 2, 3, 4, 5, 6, 7, 8],
+                                 size=2000, n_stars=8)
+
+    # Bright star but class 1, not picked
+    stars.add_fake_star(mag=6.5, row=row_max - 1, col=col_max - 1, id=10, CLASS=1)
+
+    # Bright star just off the FOV, not picked
+    stars.add_fake_star(mag=6.5, row=row_max + 1, col=col_max + 1, id=20)
+
+    # Make sure baseline catalog is working like expected
+    guides = get_guide_catalog(**STD_INFO, stars=stars)
+    assert np.all(guides['id'] == [1, 2, 3, 4, 5])
+
+    # Picking the class=1 star is fine
+    guides = get_guide_catalog(**STD_INFO, stars=stars, include_ids=10)
+    assert np.all(sorted(guides['id']) == [1, 2, 3, 4, 10])
+
+    # Picking the star off the CCD generates an exception
+    with pytest.raises(ValueError) as err:
+        get_guide_catalog(**STD_INFO, stars=stars, include_ids=20)
+    assert 'cannot include star id=20' in str(err)
+
 @pytest.mark.parametrize('dither', dither_cases)
 def test_edge_star(dither):
     """