Merge pull request #1038 from vsbuffalo/friendly-message-refactor

refactored the message code into a function

stdpopsim/species.py

@@ -14,6 +14,21 @@
 registered_species = {}
 
 
+def _missing_from_catalog(type_of_thing, thing_id, available_things):
+    """
+    Return a user-friendly message if the user requests an identifier not in the
+    catalog
+
+    :param str type_of_thing: The kind of requested object (e.g. species,
+         genetic map, etc.)
+    :param str thing_id: the string identifier of the requested object.
+    :param list available_things: a list of string identifiers that are
+         available.
+    """
+    avail_str = ", ".join(available_things)
+    return f"{type_of_thing} '{thing_id}' not in catalog ({avail_str})"
+
+
 def register_species(species):
     """
     Registers the specified ``species``.
@@ -40,8 +55,7 @@ def get_species(id):
     if id not in registered_species:
         # TODO we should probably have a custom exception here and standardise
         # on using these for all the catalog search functions.
-        avail_sps_str = ", ".join(registered_species)
-        raise ValueError(f"Species '{id}' not in catalog ({avail_sps_str})")
+        raise ValueError(_missing_from_catalog("Species", id, registered_species))
     return registered_species[id]
 
 
@@ -222,9 +236,10 @@ def get_demographic_model(self, id):
             if model.id == id:
                 return model
         available_models = [dm.id for dm in self.demographic_models]
-        avail_models_str = ", ".join(available_models)
         raise ValueError(
-            f"DemographicModel '{self.id}/{id}' not in catalog ({avail_models_str})"
+            _missing_from_catalog(
+                "DemographicModel", f"{self.id}/{id}", available_models
+            )
         )
 
     def add_demographic_model(self, model):
@@ -250,8 +265,9 @@ def get_dfe(self, id):
             if dfe.id == id:
                 return dfe
         available_dfes = [d.id for d in self.dfes]
-        avail_dfes_str = ", ".join(available_dfes)
-        raise ValueError(f"DFE '{self.id}/{id}' not in catalog ({avail_dfes_str})")
+        raise ValueError(
+            _missing_from_catalog("DFE", f"{self.id}/{id}", available_dfes)
+        )
 
     def add_dfe(self, dfe):
         if dfe.id in [d.id for d in self.dfes]:
@@ -281,9 +297,8 @@ def get_genetic_map(self, id):
             if gm.id == id:
                 return gm
         available_maps = [gm.id for gm in self.genetic_maps]
-        avail_maps_str = ", ".join(available_maps)
         raise ValueError(
-            f"Genetic map '{self.id}/{id}' not in catalog ({avail_maps_str})"
+            _missing_from_catalog("GeneticMap", f"{self.id}/{id}", available_maps)
         )
 
     def add_annotations(self, annotations):
@@ -309,7 +324,6 @@ def get_annotations(self, id):
             if an.id == id:
                 return an
         available_anno = [anno.id for anno in self.annotations]
-        avail_anno_str = ", ".join(available_anno)
         raise ValueError(
-            f"Annotations '{self.id}/{id}' not in catalog ({avail_anno_str})"
+            _missing_from_catalog("Annotations", f"{self.id}/{id}", available_anno)
         )

tests/test_cli.py

@@ -769,30 +769,27 @@ class TestSearchWrappers:
     def test_bad_species(self):
         with mock.patch("stdpopsim.cli.exit", autospec=True) as mocked_exit:
             cli.get_species_wrapper("XXX")
-            avail_sps_str = ", ".join(stdpopsim.species.registered_species)
+            available_species = ", ".join(stdpopsim.species.registered_species)
             mocked_exit.assert_called_once_with(
-                f"Species 'XXX' not in catalog ({avail_sps_str})"
+                f"Species 'XXX' not in catalog ({available_species})"
             )
 
     def test_bad_model(self):
         species = stdpopsim.get_species("HomSap")
         with mock.patch("stdpopsim.cli.exit", autospec=True) as mocked_exit:
             cli.get_model_wrapper(species, "XXX")
-            available_models = [dm.id for dm in species.demographic_models]
-            avail_models_str = ", ".join(available_models)
+            available_models = ", ".join([dm.id for dm in species.demographic_models])
             mocked_exit.assert_called_once_with(
-                f"DemographicModel 'HomSap/XXX' not in catalog ({avail_models_str})"
+                f"DemographicModel 'HomSap/XXX' not in catalog ({available_models})"
             )
 
     def test_bad_genetic_map(self):
         species = stdpopsim.get_species("HomSap")
         with mock.patch("stdpopsim.cli.exit", autospec=True) as mocked_exit:
             cli.get_genetic_map_wrapper(species, "XXX")
-            available_maps = [gm.id for gm in species.genetic_maps]
-            avail_maps_str = ", ".join(available_maps)
-
+            available_maps = ", ".join([gm.id for gm in species.genetic_maps])
             mocked_exit.assert_called_once_with(
-                f"Genetic map 'HomSap/XXX' not in catalog ({avail_maps_str})"
+                f"GeneticMap 'HomSap/XXX' not in catalog ({available_maps})"
             )