Repository: dsavransky/EXOSIMS

Issue:
A number of TargetList filters are never used
**Describe the bug**
All of these filters have no entry point:
- life_expectancy_filter
- main_sequence_filter
- fgk_filter
- vis_mag_filter
- max_dmag_filter

Are these supposed to have Boolean flags that enable them?

**To Reproduce**
N/A

**Expected behavior**
The TargetList prototype should have an input flag that allows the user to turn on these filters and default settings.

**JSON script:**
N/A

**EXOSIMS version:**
 - Current master

**Additional context**
I ran into this while investigating how stars that have angular diameters much larger than the detection mode's inner working angle are reaching the calc_dMag_per_intTime step (Aldebaran is one example and causes issues since it's too large for some OS interpolants). I added that as a filter locally since those are a waste of time to observe which I can make a pull for.

Discussion:
@dsavransky: These are part of the original filter set defined by Maggie Turnbull for EXOCAT1.  A few of them proved to be redundant, which is why they were toggled off by default, but I agree that if they're in the code, they should have associated booleans that allow them to be toggled. We should review the functionality of each one as well. 

Solution patch:
diff --git a/EXOSIMS/Prototypes/OpticalSystem.py b/EXOSIMS/Prototypes/OpticalSystem.py
index 86174c42..bd4ff2f3 100644
--- a/EXOSIMS/Prototypes/OpticalSystem.py
+++ b/EXOSIMS/Prototypes/OpticalSystem.py
@@ -581,7 +581,7 @@ def populate_scienceInstruments(self, scienceInstruments):
                 warnings.warn(
                     f'Input FoV ({inst["FoV"]}) is larger than FoV computed '
                     f"from pixelScale ({predFoV.to(u.arcsec) :.2f}) for "
-                    f'instrument {inst["name"]}. This feels like a mistkae.'
+                    f'instrument {inst["name"]}. This feels like a mistake.'
                 )
 
             # parameters specific to spectrograph
diff --git a/EXOSIMS/Prototypes/TargetList.py b/EXOSIMS/Prototypes/TargetList.py
index c5b71d15..71e55fed 100644
--- a/EXOSIMS/Prototypes/TargetList.py
+++ b/EXOSIMS/Prototypes/TargetList.py
@@ -225,6 +225,9 @@ class TargetList(object):
             Base longitude of the ascending node for target system orbital planes
         OpticalSystem (:ref:`OpticalSystem`):
             :ref:`OpticalSystem` object
+        optional_filters (dict):
+            Dictionary of optional filters to apply to the target list.  Keys are the
+            filter's name and values are the values necessary to apply the filter.
         parx (astropy.units.quantity.Quantity):
             Parallaxes
         PlanetPhysicalModel (:ref:`PlanetPhysicalModel`):
@@ -326,6 +329,7 @@ def __init__(
         cherryPickStars=None,
         skipSaturationCalcs=True,
         massLuminosityRelationship="Henry1993",
+        optional_filters={},
         **specs,
     ):
         # start the outspec
@@ -359,6 +363,29 @@ def __init__(
             "Fang2010",
         ]
 
+        # Set up optional filters
+        default_filters = {
+            "outside_IWA_filter": {"enabled": True},
+            "completeness_filter": {"enabled": True},
+        }
+        # Add the binary filter to the default optional filters
+        if optional_filters.get("binary_filter"):
+            # if binary_filter is provided in the optional_filters dict, we use that
+            # value but raise a warning if it conflicts with the set filter value.
+            if self.filterBinaries != optional_filters.get("enabled"):
+                warnings.warn(
+                    f"binary_filter is {optional_filters.get('enabled')} "
+                    f"but filterBinaries is {self.filterBinaries}. "
+                    f"Using binary_filter value of {optional_filters.get('enabled')}."
+                )
+        else:
+            default_filters["binary_filter"] = {"enabled": self.filterBinaries}
+        # Add the provided optional filters to the default filters, overriding
+        # the defaults if necessary, and then save the combined dictionary as a
+        # class attribute
+        default_filters.update(optional_filters)
+        self.optional_filters = default_filters
+
         assert (
             self.massLuminosityRelationship in allowable_massLuminosityRelationships
         ), (
@@ -530,6 +557,15 @@ def __init__(
         self.blackbody_spectra = np.ndarray((self.nStars), dtype=object)
         self.catalog_atts.append("blackbody_spectra")
 
+        # Apply optional filters that don't depend on completeness
+        secondary_filter_names = ["completeness_filter"]
+        first_filter_set = {
+            key: self.optional_filters.get(key, {})
+            for key in self.optional_filters.keys()
+            if key not in secondary_filter_names
+        }
+        self.filter_target_list(first_filter_set)
+
         # Calculate saturation and intCutoff delta mags and completeness values
         self.calc_saturation_and_intCutoff_vals()
 
@@ -540,8 +576,11 @@ def __init__(
         # generate any completeness update data needed
         self.Completeness.gen_update(self)
 
-        # apply any requested additional filters
-        self.filter_target_list(**specs)
+        # apply any requested additional filters that depend on completeness
+        second_filter_set = {
+            key: self.optional_filters.get(key, {}) for key in secondary_filter_names
+        }
+        self.filter_target_list(second_filter_set)
 
         # if we're doing filter_for_char, then that means that we haven't computed the
         # star fluxes for the detection mode.  Let's do that now (post-filtering to
@@ -1346,34 +1385,72 @@ def fillPhotometryVals(self):
                         + mag_to_add[i]
                     )
 
-    def filter_target_list(self, **specs):
+    def filter_target_list(self, filters):
         """This function is responsible for filtering by any required metrics.
 
-        The prototype implementation removes the following stars:
-            * Stars with NAN values in their parameters
-            * Binary stars
-            * Systems with planets inside the OpticalSystem fundamental IWA
-            * Systems where minimum integration time is longer than OpticalSystem cutoff
-            * Systems not meeting the Completeness threshold
-
-        Additional filters can be provided in specific TargetList implementations.
+        This should be used for *optional* filters. The ones in the prototype
+        are:
+            binary_filter
+            outside_IWA_filter
+            life_expectancy_filter
+            main_sequence_filter
+            fgk_filter
+            vis_mag_filter (takes Vmagcrit as input)
+            max_dmag_filter
+            completeness_filter (Has to be run after the completeness values are calculated)
+            vmag_filter (takes vmag_range as input)
+            ang_diam_filter
 
+        Args:
+            filters (dict):
+                Dictionary of filters to apply to the target list.  Keys are
+                filter names, and values are the filter functions to apply.
+                Looks like:
+                filters = {
+                    "binary_filter": {"enabled": True},
+                    "outside_IWA_filter": {"enabled": True},
+                    vmag_filter: {"enabled": True, "params": {"vmag_range": [4, 10]}},
+                }
         """
-        # filter out binary stars
-        if self.filterBinaries:
-            self.binary_filter()
-            if self.explainFiltering:
-                print("%d targets remain after binary filter." % self.nStars)
+        for filter_name, filter_config in filters.items():
+            # check if the filter is enabled
+            if filter_config.get("enabled", False):
+                # get the filter function
+                if hasattr(self, filter_name):
+                    # Get the filter method
+                    filter_func = getattr(self, filter_name)
+                    # Apply the filter
+                    filter_func(**filter_config.get("params", {}))
+                    if self.explainFiltering:
+                        print(
+                            f"{self.nStars} targets remain after {filter_name.replace('_', ' ')}."
+                        )
+                else:
+                    raise AttributeError(
+                        (f"No filter '{filter_name}' in {self.__class__.__name__}.")
+                    )
 
-        # filter out systems with planets within the IWA
-        self.outside_IWA_filter()
-        if self.explainFiltering:
-            print("%d targets remain after IWA filter." % self.nStars)
+    def vmag_filter(self, vmag_range):
+        """Removes stars with Vmag outside of specified range
 
-        # filter out systems which do not reach the completeness threshold
-        self.completeness_filter()
-        if self.explainFiltering:
-            print("%d targets remain after completeness filter." % self.nStars)
+        Args:
+            vmag_range (list):
+                2-element list of min, max Vmag values
+
+        """
+        meets_lower_bound = self.Vmag > vmag_range[0]
+        meets_upper_bound = self.Vmag < vmag_range[1]
+        i = np.where(meets_lower_bound & meets_upper_bound)[0]
+        self.revise_lists(i)
+
+    def ang_diam_filter(self):
+        """Remove stars which are larger than the IWA."""
+        # angular radius of each star
+        ang_rad = self.diameter / 2
+        IWA = self.OpticalSystem.IWA
+        # indices of stars with angular radius less than IWA
+        i = np.where(ang_rad < IWA)[0]
+        self.revise_lists(i)
 
     def nan_filter(self):
         """Filters out targets where required values are nan"""
diff --git a/EXOSIMS/StarCatalog/SIMBADCatalog.py b/EXOSIMS/StarCatalog/SIMBADCatalog.py
index 7277c95f..5ac0fcaa 100644
--- a/EXOSIMS/StarCatalog/SIMBADCatalog.py
+++ b/EXOSIMS/StarCatalog/SIMBADCatalog.py
@@ -131,7 +131,7 @@ def SIMBAD_mat2pkl(self, matpath, pklpath):
                     bc = x.BINARY_CUT.tolist()
                     y["Binary_Cut"] = [False] * len(bc)
                     for i in range(len(bc)):
-                        if bc[i] == "cut":
+                        if isinstance(bc[i], str) and bc[i] == "cut":
                             y["Binary_Cut"][i] = True
                 else:
                     y[mat2pkl[field]] = getattr(x, field).tolist()
diff --git a/EXOSIMS/TargetList/KnownRVPlanetsTargetList.py b/EXOSIMS/TargetList/KnownRVPlanetsTargetList.py
index aaf0c98d..3cfac52c 100644
--- a/EXOSIMS/TargetList/KnownRVPlanetsTargetList.py
+++ b/EXOSIMS/TargetList/KnownRVPlanetsTargetList.py
@@ -21,7 +21,6 @@ class KnownRVPlanetsTargetList(TargetList):
     """
 
     def __init__(self, **specs):
-
         # define mapping between attributes we need and the IPAC data
         # table loaded in the Planet Population module
         self.atts_mapping = {
@@ -171,7 +170,7 @@ def populate_target_list(self, **specs):
         self.Binary_Cut = np.zeros(self.nStars, dtype=bool)
         self.hasKnownPlanet = np.ones(self.nStars, dtype=bool)
 
-    def filter_target_list(self, **specs):
+    def filter_target_list(self, filters):
         """Filtering is done as part of populating the table, so this
         method is overloaded to do nothing.
         """
diff --git a/requirements.txt b/requirements.txt
index cc7caeff..9dd11215 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -4,7 +4,7 @@ astropy>=5.0.0
 jplephem>=2.20.0
 ortools>=9.0
 h5py>=3.7.0
-astroquery>=0.4.6
+astroquery==0.4.6
 exo-det-box
 tqdm>=4.59
 pandas>=1.3


Test patch:
diff --git a/tests/TargetList/test_KnownRVPlanetsTargetList.py b/tests/TargetList/test_KnownRVPlanetsTargetList.py
index 3ec4216f..70ad5492 100644
--- a/tests/TargetList/test_KnownRVPlanetsTargetList.py
+++ b/tests/TargetList/test_KnownRVPlanetsTargetList.py
@@ -303,7 +303,7 @@ def test_filter_target_list(self):
         """
         tlist = self.fixture
         keys = sorted(list(tlist.__dict__))  # makes a copy
-        tlist.filter_target_list()
+        tlist.filter_target_list({})
         self.assertEqual(tlist._modtype, "TargetList")
         # just ensure the same keys are still present
         self.assertListEqual(keys, sorted(list(tlist.__dict__)))