DM-40151: Fix bug in ImportTestCase

.github/workflows/rebase_checker.yaml

@@ -1,4 +1,3 @@
----
 name: Check that 'main' is not merged into the development branch
 
 on: pull_request

python/lsst/utils/classes.py

@@ -19,7 +19,7 @@
 
 import functools
 from collections.abc import Callable
-from typing import Any, Type, TypeVar
+from typing import Any, ClassVar, Type, TypeVar
 
 
 class Singleton(type):
@@ -33,7 +33,7 @@ class Singleton(type):
     adjust state of the singleton.
     """
 
-    _instances: dict[type, Any] = {}
+    _instances: ClassVar[dict[type, Any]] = {}
 
     # Signature is intentionally not substitutable for type.__call__ (no *args,
     # **kwargs) to require classes that use this metaclass to have no
@@ -87,7 +87,7 @@ def __getstate__(self: _T) -> dict:  # noqa: N807
         # Disable default state-setting when unpickled.
         return {}
 
-    cls.__getstate__ = __getstate__
+    cls.__getstate__ = __getstate__  # type: ignore[assignment]
 
     def __setstate__(self: _T, state: Any) -> None:  # noqa: N807
         # Disable default state-setting when copied.

python/lsst/utils/packages.py

@@ -14,6 +14,7 @@
 
 from __future__ import annotations
 
+import contextlib
 import hashlib
 import importlib
 import io
@@ -27,7 +28,7 @@
 import types
 from collections.abc import Mapping
 from functools import lru_cache
-from typing import Any
+from typing import Any, ClassVar
 
 import yaml
 
@@ -115,10 +116,9 @@ def getPythonPackages() -> dict[str, str]:
     """
     # Attempt to import libraries that only report their version in python
     for module_name in PYTHON:
-        try:
+        # If it's not available we continue.
+        with contextlib.suppress(Exception):
             importlib.import_module(module_name)
-        except Exception:
-            pass  # It's not available, so don't care
 
     packages = {"python": sys.version}
 
@@ -410,7 +410,12 @@ class Packages(dict):
     This is a wrapper around a dict with some convenience methods.
     """
 
-    formats = {".pkl": "pickle", ".pickle": "pickle", ".yaml": "yaml", ".json": "json"}
+    formats: ClassVar[dict[str, str]] = {
+        ".pkl": "pickle",
+        ".pickle": "pickle",
+        ".yaml": "yaml",
+        ".json": "json",
+    }
 
     def __setstate__(self, state: dict[str, Any]) -> None:
         # This only seems to be called for old pickle files where

python/lsst/utils/tests.py

@@ -32,7 +32,6 @@
 import contextlib
 import functools
 import gc
-import importlib.resources as resources
 import inspect
 import itertools
 import os
@@ -44,6 +43,7 @@
 import unittest
 import warnings
 from collections.abc import Callable, Iterable, Iterator, Mapping, Sequence
+from importlib import resources
 from typing import Any, ClassVar
 
 import numpy
@@ -129,7 +129,7 @@
 class MemoryTestCase(unittest.TestCase):
     """Check for resource leaks."""
 
-    ignore_regexps: list[str] = []
+    ignore_regexps: ClassVar[list[str]] = []
     """List of regexps to ignore when checking for open files."""
 
     @classmethod
@@ -367,7 +367,7 @@
         for mod in cls.PACKAGES:
             test_name = "test_import_" + mod.replace(".", "_")
 
-            def test_import(*args: Any) -> None:
+            def test_import(*args: Any, mod=mod) -> None:
                 self = args[0]
                 self.assertImport(mod)
 
@@ -378,7 +378,7 @@
         # If there are no packages listed that is likely a mistake and
         # so register a failing test.
         if cls._n_registered == 0:
-            setattr(cls, "test_no_packages_registered", cls._test_no_packages_registered_for_import_testing)
+            cls.test_no_packages_registered = cls._test_no_packages_registered_for_import_testing
 
     def assertImport(self, root_pkg):
         for file in resources.files(root_pkg).iterdir():
@@ -473,10 +473,8 @@
         # Use stacklevel 3 so that the warning is reported from the end of the
         # with block
         warnings.warn(f"Unexpectedly found pre-existing tempfile named {outPath!r}", stacklevel=3)
-        try:
+        with contextlib.suppress(OSError):
             os.remove(outPath)
-        except OSError:
-            pass
 
     yield outPath
 

python/lsst/utils/timer.py

@@ -22,7 +22,7 @@
 import logging
 import time
 from collections.abc import Callable, Collection, Iterable, Iterator, MutableMapping
-from contextlib import contextmanager
+from contextlib import contextmanager, suppress
 from typing import TYPE_CHECKING, Any
 
 from astropy import units as u
@@ -109,15 +109,13 @@ def logPairs(
     """
     if obj is not None:
         if metadata is None:
-            try:
+            with suppress(AttributeError):
                 metadata = obj.metadata
-            except AttributeError:
-                pass
+
         if logger is None:
-            try:
+            with suppress(AttributeError):
                 logger = obj.log
-            except AttributeError:
-                pass
+
     strList = []
     for name, value in pairs:
         if metadata is not None:
@@ -197,10 +195,9 @@ def logInfo(
     * Version 1: ``MaxResidentSetSize`` will be stored in bytes.
     """
     if metadata is None and obj is not None:
-        try:
+        with suppress(AttributeError):
             metadata = obj.metadata
-        except AttributeError:
-            pass
+
     if metadata is not None:
         # Log messages already have timestamps.
         utcStr = datetime.datetime.utcnow().isoformat()

python/lsst/utils/wrappers.py

@@ -307,20 +307,14 @@ def __subclasscheck__(cls, subclass):
         # any registered type or true subclass thereof.
         if subclass in cls._registry.values():
             return True
-        for v in cls._registry.values():
-            if issubclass(subclass, v):
-                return True
-        return False
+        return any(issubclass(subclass, v) for v in cls._registry.values())
 
     def __instancecheck__(cls, instance):
         # Special method hook for the isinstance built-in: we return true for
         # an instance of any registered type or true subclass thereof.
         if type(instance) in cls._registry.values():
             return True
-        for v in cls._registry.values():
-            if isinstance(instance, v):
-                return True
-        return False
+        return any(isinstance(instance, v) for v in cls._registry.values())
 
     def __subclasses__(cls):
         """Return a tuple of all classes that inherit from this class."""
@@ -396,9 +390,7 @@ def setattrSafe(name, value):
                 setattrSafe(p, k)
         else:
             raise ValueError(
-                "key must have {} elements (one for each of {})".format(
-                    len(cls.TEMPLATE_PARAMS), cls.TEMPLATE_PARAMS
-                )
+                f"key must have {len(cls.TEMPLATE_PARAMS)} elements (one for each of {cls.TEMPLATE_PARAMS})"
             )
 
         for name, attr in cls._inherited.items():

tests/test_getTempFilePath.py

@@ -36,9 +36,8 @@ def testBasics(self):
             # Path will have unique component so do not test full equality
             self.assertIn("test_getTempFilePath_testBasics", tmpFile)
             self.assertTrue(tmpFile.endswith(".txt"))
-            f = open(tmpFile, "w")
-            f.write("foo\n")
-            f.close()
+            with open(tmpFile, "w") as f:
+                f.write("foo\n")
         self.assertFalse(os.path.exists(tmpFile))
 
     def testMultipleCallDepth(self):
@@ -53,9 +52,8 @@ def runGetTempFile(self, funcName):
             # Path will have unique component so do not test full equality
             self.assertIn(f"test_getTempFilePath_{funcName}", tmpFile)
             self.assertTrue(tmpFile.endswith(".fits"))
-            f = open(tmpFile, "w")
-            f.write("foo\n")
-            f.close()
+            with open(tmpFile, "w") as f:
+                f.write("foo\n")
         self.assertFalse(os.path.exists(tmpFile))
 
     def runLevel2(self, funcName):

tests/test_timer.py

@@ -328,7 +328,7 @@ def test_profile(self):
             self.assertEqual(len(cm.output), 2)
             self.assertIsNotNone(prof)
             self.assertTrue(os.path.exists(tmp.name))
-            self.assertIsInstance(pstats.Stats(tmp.name), pstats.Stats),
+            self.assertIsInstance(pstats.Stats(tmp.name), pstats.Stats)
 
 
 if __name__ == "__main__":