Merge pull request #125 from lsst-sitcom/tickets/DM-47189

DM-47189: Write default butler factory function for all instruments

mypy.ini

@@ -4,6 +4,9 @@ warn_unused_configs = False
 warn_redundant_casts = False
 plugins = pydantic.mypy
 
+exclude = (?x)
+    ^python/lsst/summit/utils/m1m3/inertia_compensation_system\.py$
+
 [mypy-requests.*]
 ignore_missing_imports = True
 

python/lsst/summit/utils/butlerUtils.py

@@ -21,16 +21,19 @@
 
 import copy
 import itertools
+import logging
 from collections.abc import Iterable, Mapping
 from typing import Any
 
 from deprecated.sphinx import deprecated
 
 import lsst.daf.butler as dafButler
 from lsst.summit.utils.utils import getSite
+from lsst.utils.iteration import ensure_iterable
 
 __all__ = [
-    "makeDefaultLatissButler",
+    "makeDefaultLatissButler",  # deprecated
+    "makeDefaultButler",
     "updateDataId",
     "sanitizeDayObs",
     "getMostRecentDayObs",
@@ -40,7 +43,7 @@
     "getDayObs",
     "getSeqNum",
     "getExpId",
-    "datasetExists",
+    "datasetExists",  # deprecated
     "sortRecordsByDayObsThenSeqNum",
     "getDaysWithData",
     "getExpIdFromDayObsSeqNum",
@@ -109,6 +112,11 @@ def _update_RECENT_DAY(day: int) -> None:
     RECENT_DAY = max(day - 1, RECENT_DAY)
 
 
+@deprecated(
+    reason="Use the more generic makeDefaultButler('LATISS'). Will be removed after v28.0.",
+    version="v27.0",
+    category=FutureWarning,
+)
 def makeDefaultLatissButler(
     *,
     extraCollections: list[str] | None = None,
@@ -132,14 +140,75 @@ def makeDefaultLatissButler(
     butler : `lsst.daf.butler.Butler`
         The butler.
     """
-    # TODO: Add logging to which collections are going in
-    collections = getLatissDefaultCollections()
-    if extraCollections:
-        collections.extend(extraCollections)
+    return makeDefaultButler(
+        "LATISS", extraCollections=extraCollections, writeable=writeable, embargo=embargo
+    )
+
+
+def makeDefaultButler(
+    instrument: str,
+    *,
+    extraCollections: list[str] | None = None,
+    writeable: bool = False,
+    embargo: bool = True,
+) -> dafButler.Butler:
+    """Create a butler for the instrument using default collections, regardless
+    of the location.
+
+    Parameters
+    ----------
+    extraCollections : `list` of `str`
+        Extra input collections to supply to the butler init.
+    writable : `bool`, optional
+        Whether to make a writable butler.
+    embargo : `bool`, optional
+        Use the embargo repo instead of the main one. Needed to access
+        embargoed data if not at a summit-like location.
+
+    Returns
+    -------
+    butler : `lsst.daf.butler.Butler`
+        The butler.
+
+    Raises
+    ------
+    FileNotFoundError
+        Raised if the butler cannot be created.
+    """
+    SUPPORTED_SITES = [
+        "summit",
+        "tucson",
+        "base",
+        "staff-rsp",
+        "rubin-devl",
+        "usdf-k8s",
+    ]
+
+    site = getSite()
+    if site not in SUPPORTED_SITES:
+        raise ValueError(f"Default butler creation only supported at sites: {SUPPORTED_SITES}, got {site=}")
+
+    summitLike = site in ["summit", "tucson", "base"]
+    if summitLike:
+        if embargo is True:
+            logger = logging.getLogger(__name__)
+            logger.debug("embargo option is irrelevant on the summit, ignoring")
+            embargo = False  # there's only one repo too, so this makes the code more simple too
+
+    baseCollection = f"{instrument}/defaults"
+    raCollection = f"{instrument}/quickLook" if summitLike else f"{instrument}/nightlyValidation"
+
+    repo = instrument if embargo is False else "embargo"
+
+    collections = [baseCollection, raCollection]
+    if extraCollections is not None:
+        assert extraCollections is not None  # just for mypy
+        extraCollectionsList = ensure_iterable(extraCollections)
+        collections.extend(extraCollectionsList)
+
     try:
-        repoString = "LATISS" if not embargo else "/repo/embargo"
         butler = dafButler.Butler.from_config(
-            repoString, collections=collections, writeable=writeable, instrument="LATISS"
+            repo, collections=collections, writeable=writeable, instrument=instrument
         )
     except (FileNotFoundError, RuntimeError):
         # Depending on the value of DAF_BUTLER_REPOSITORY_INDEX and whether

python/lsst/summit/utils/efdUtils.py

@@ -829,11 +829,10 @@ def getCommands(
                 case "astropy":
                     timeKey = Time(time)
                 case "python":
+                    assert isinstance(time, pd.Timestamp)
                     timeKey = time.to_pydatetime()
 
             if timeKey in commandTimes:
-                raise ValueError(
-                    f"There is already a command at {timeKey=} -" " make a better data structure!"
-                )
+                raise ValueError(f"There is already a command at {timeKey=} - make a better data structure!")
             commandTimes[timeKey] = command
     return commandTimes

python/lsst/summit/utils/tmaUtils.py

@@ -151,7 +151,7 @@ def getAzimuthElevationDataForEvent(
     event: TMAEvent,
     prePadding: float = 0,
     postPadding: float = 0,
-) -> tuple[pd.Dataframe, pd.Dataframe]:
+) -> tuple[pd.DataFrame, pd.DataFrame]:
     """Get the data for the az/el telemetry topics for a given TMAEvent.
 
     The error between the actual and demanded positions is calculated and added
@@ -406,6 +406,9 @@ def getPlotTime(time: pd.Timestamp | Time | datetime.datetime) -> datetime.datet
             client, event, prePadding=prePadding, postPadding=postPadding
         )
 
+    assert azimuthData is not None
+    assert elevationData is not None
+
     # Use the native color cycle for the lines. Because they're on different
     # axes they don't cycle by themselves
     lineColors = [p["color"] for p in plt.rcParams["axes.prop_cycle"]]

tests/test_bestEffortIsr.py

@@ -33,7 +33,7 @@ class BestEffortIsrTestCase(lsst.utils.tests.TestCase):
     def setUpClass(cls):
         try:
             cls.bestEffortIsr = BestEffortIsr()
-        except FileNotFoundError:
+        except (FileNotFoundError, ValueError):
             raise unittest.SkipTest("Skipping tests that require the LATISS butler repo.")
 
         # chosen as this is available in the following locations - collections:

tests/test_butlerUtils.py

@@ -55,6 +55,7 @@
     getMostRecentDayObs,
     getSeqNum,
     getSeqNumsForDayObs,
+    getSite,
     makeDefaultLatissButler,
     sanitizeDayObs,
     sortRecordsByDayObsThenSeqNum,
@@ -73,6 +74,8 @@ def setUp(self):
 
         # butler stuff
         try:
+            if getSite() == "jenkins":
+                raise unittest.SkipTest("Skip running butler-driven tests in Jenkins.")
             self.butler = makeDefaultLatissButler()
         except FileNotFoundError:
             raise unittest.SkipTest("Skipping tests that require the LATISS butler repo.")
@@ -454,6 +457,8 @@ def test_makeDefaultLatissButlerRaiseTypes(self):
         """makeDefaultLatissButler unifies the mixed exception types from
         butler inits, so test all available possibilities here.
         """
+        if getSite() == "jenkins":
+            raise unittest.SkipTest("Skip running butler-driven tests in Jenkins.")
         with unittest.mock.patch.dict("os.environ"):
             if "DAF_BUTLER_REPOSITORY_INDEX" in os.environ:  # can't del unless it's already there
                 del os.environ["DAF_BUTLER_REPOSITORY_INDEX"]

tests/test_imageExaminer.py

@@ -27,12 +27,15 @@
 from lsst.summit.utils import ImageExaminer
 from lsst.summit.utils.bestEffort import BestEffortIsr
 from lsst.summit.utils.butlerUtils import makeDefaultLatissButler
+from lsst.summit.utils.utils import getSite
 
 
 class ImageExaminerTestCase(lsst.utils.tests.TestCase):
     @classmethod
     def setUpClass(cls):
         try:
+            if getSite() == "jenkins":
+                raise unittest.SkipTest("Skip running butler-driven tests in Jenkins.")
             cls.butler = makeDefaultLatissButler()
         except FileNotFoundError:
             raise unittest.SkipTest("Skipping tests that require the LATISS butler repo.")

tests/test_nightReport.py

@@ -36,12 +36,15 @@
 
 import lsst.summit.utils.butlerUtils as butlerUtils  # noqa: E402
 from lsst.summit.utils.nightReport import ColorAndMarker, NightReport  # noqa: E402
+from lsst.summit.utils.utils import getSite  # noqa: E402
 
 
 class NightReportTestCase(lsst.utils.tests.TestCase):
     @classmethod
     def setUpClass(cls):
         try:
+            if getSite() == "jenkins":
+                raise unittest.SkipTest("Skip running butler-driven tests in Jenkins.")
             cls.butler = butlerUtils.makeDefaultLatissButler()
         except FileNotFoundError:
             raise unittest.SkipTest("Skipping tests that require the LATISS butler repo.")