Merge pull request #135 from lsst/tickets/DM-48788

DM-48788: Use ruff format

Author: timj

.github/workflows/lint.yaml

@@ -2,28 +2,16 @@ repos:
   - repo: https://github.com/pre-commit/pre-commit-hooks
     rev: v5.0.0
     hooks:
-      - id: check-toml
       - id: check-yaml
         args:
           - "--unsafe"
       - id: end-of-file-fixer
       - id: trailing-whitespace
-  - repo: https://github.com/psf/black
-    rev: 25.1.0
-    hooks:
-      - id: black
-        # It is recommended to specify the latest version of Python
-        # supported by your project here, or alternatively use
-        # pre-commit's default_language_version, see
-        # https://pre-commit.com/#top_level-default_language_version
-        language_version: python3.11
-  - repo: https://github.com/pycqa/isort
-    rev: 6.0.0
-    hooks:
-      - id: isort
-        name: isort (python)
+      - id: check-toml
   - repo: https://github.com/astral-sh/ruff-pre-commit
     # Ruff version.
     rev: v0.9.4
     hooks:
       - id: ruff
+        args: [--fix]
+      - id: ruff-format

.pre-commit-config.yaml

@@ -5,6 +5,7 @@ target-version = ["py311"]
 [tool.isort]
 profile = "black"
 line_length = 110
+known_first_party = ["lsst"]
 
 [tool.ruff]
 line-length = 110
@@ -28,11 +29,18 @@ select = [
     "F",  # pyflakes
     "N",  # pep8-naming
     "W",  # pycodestyle
+    "I",  # isort
+    "RUF022",  # Sort __all__
+    "C4",  # comprehensions
+    "B",  # bugbear
 ]
 extend-select = [
     "RUF100", # Warn about unused noqa
 ]
 
+[tool.lint.isort]
+known-first-party = ["lsst"]
+
 [tool.ruff.lint.pycodestyle]
 max-doc-length = 79
 
@@ -41,3 +49,8 @@ convention = "numpy"
 
 [tool.pytest.ini_options]
 addopts = "--import-mode=importlib"  # Recommended as best practice
+
+[tool.ruff.format]
+docstring-code-format = true
+# ruff format does not seem to take indenting into account.
+docstring-code-line-length = 69

pyproject.toml

@@ -2,7 +2,7 @@
 class.
 """
 
-__all__ = ("filesToTag", "DoxygenBuilder")
+__all__ = ("DoxygenBuilder", "filesToTag")
 
 import csv
 import fnmatch
@@ -292,7 +292,7 @@ def ProductDir(env, product):
 
     global _productDirs
     try:
-        _productDirs
+        _productDirs  # noqa: B018
     except Exception:
         try:
             _productDirs = eupsForScons.productDir(eupsenv=eupsForScons.getEups())
@@ -431,7 +431,7 @@ def _quote_paths(pathList):
         outConfigFile.write(f"FILE_PATTERNS = {' '.join(self.patterns)}\n")
         outConfigFile.write("RECURSIVE = YES\n" if self.recursive else "RECURSIVE = NO\n")
         allOutputs = {"html", "latex", "man", "rtf", "xml"}
-        for output, path in zip(self.outputs, self.outputPaths):
+        for output, path in zip(self.outputs, self.outputPaths, strict=True):
             try:
                 allOutputs.remove(output.lower())
             except Exception:
@@ -746,7 +746,7 @@ def makeRecordFile(target, source, env):
         # Do not attempt to write hashes and file sizes since these can
         # change if the package is not really installed into an EUPS tree.
         all_files = set()
-        for root, dirs, files in os.walk(pythonDir):
+        for root, _, files in os.walk(pythonDir):
             root = root.removeprefix(pythonDir)
             root = root.removeprefix("/")
             all_files.update({os.path.join(root, f) for f in files})
@@ -833,7 +833,7 @@ def makePythonScript(target, source, env):
             os.chmod(cmdfile, newmode)
 
     results = []
-    for cmd, code in scripts.items():
+    for cmd in scripts:
         filename = f"bin/{cmd}"
 
         # Do not do anything if there is an equivalent target in bin.src

python/lsst/sconsUtils/builders.py

@@ -167,9 +167,9 @@ class Configuration:
         name ``<root>/doc/<name>.inc``.
     hasDoxygenTag : `bool`, optional
         If True, the package generates a Doxygen TAG file.
-    includeFileDirs : `list`, optional
+    includeFileDirs : `collections.abc.Sequence`, optional
         List of directories that should be searched for include files.
-    libFileDirs : `list`, optional
+    libFileDirs : `collections.abs.Sequence`, optional
         List of directories that should be searched for libraries.
     eupsProduct : `str`
         Name of the EUPS product for the package, if different from the name
@@ -223,8 +223,8 @@ def __init__(
         headers=(),
         libs=None,
         hasSwigFiles=True,
-        includeFileDirs=["include"],
-        libFileDirs=["lib"],
+        includeFileDirs=("include",),
+        libFileDirs=("lib",),
         hasDoxygenInclude=False,
         hasDoxygenTag=True,
         eupsProduct=None,

python/lsst/sconsUtils/dependencies.py

@@ -1,12 +1,12 @@
 """Builders and path setup for installation targets."""
 
 __all__ = (
-    "makeProductPath",
+    "DirectoryInstaller",
+    "SConsUtilsEnvironment",
     "determineVersion",
     "getFingerprint",
+    "makeProductPath",
     "setPrefix",
-    "DirectoryInstaller",
-    "SConsUtilsEnvironment",
 )
 
 import glob
@@ -230,7 +230,7 @@ def Declare(self, products=None):
                 product = self["eupsProduct"]
 
             if "EUPS_DIR" in os.environ:
-                self["ENV"]["PATH"] += os.pathsep + f'{os.environ["EUPS_DIR"]}/bin'
+                self["ENV"]["PATH"] += os.pathsep + f"{os.environ['EUPS_DIR']}/bin"
                 self["ENV"]["EUPS_LOCK_PID"] = os.environ.get("EUPS_LOCK_PID", "-1")
                 if "undeclare" in SCons.Script.COMMAND_LINE_TARGETS or self.GetOption("clean"):
                     if version:
@@ -254,15 +254,15 @@ def Declare(self, products=None):
                     )
 
                     if "eupsPath" in self:
-                        command += f' -Z {self["eupsPath"]}'
+                        command += f" -Z {self['eupsPath']}"
 
                     if version:
                         command += f" {product} {version}"
 
                     current += [command + " --current"]
 
                     if self.GetOption("tag"):
-                        command += f' --tag={self.GetOption("tag")}'
+                        command += f" --tag={self.GetOption('tag')}"
 
                     declare += [command]
 
@@ -359,7 +359,7 @@ def InstallDir(self, prefix, dir, ignoreRegex=r"(~$|\.pyc$|\.os?$)", recursive=T
 
 
 @memberOf(SConsEnvironment)
-def InstallEups(env, dest, files=[], presetup=""):
+def InstallEups(env, dest, files=(), presetup=""):
     """Install a ups directory, setting absolute versions as appropriate
     (unless you're installing from the trunk, in which case no versions
     are expanded).
@@ -370,7 +370,7 @@ def InstallEups(env, dest, files=[], presetup=""):
         Environment to use.
     dest : `str`
         Destination directory.
-    files : `list`, optional
+    files : `collections.abc.Sequence`, optional
         List of files to install.  Any build/table files present in ``./ups``
         are automatically added to this list.
     presetup : `dict`, optional
@@ -389,8 +389,10 @@ def InstallEups(env, dest, files=[], presetup=""):
 
     .. code-block:: python
 
-        env.InstallEups(os.path.join(env['prefix'], "ups"),
-                        presetup={"sconsUtils" : env['version']})
+        env.InstallEups(
+            os.path.join(env["prefix"], "ups"),
+            presetup={"sconsUtils": env["version"]},
+        )
     """
     acts = []
     if not env.installing:
@@ -449,9 +451,9 @@ def InstallEups(env, dest, files=[], presetup=""):
         for i in build_obj:
             env.AlwaysBuild(i)
 
-            cmd = f'eups expandbuild -i --version {env["version"]} '
+            cmd = f"eups expandbuild -i --version {env['version']} "
             if "baseversion" in env:
-                cmd += f' --repoversion {env["baseversion"]} '
+                cmd += f" --repoversion {env['baseversion']} "
             cmd += str(i)
             eupsTargets.extend(env.AddPostAction(build_obj, env.Action(f"{cmd}", cmd)))
 

python/lsst/sconsUtils/installation.py

@@ -369,7 +369,7 @@ def rewrite_shebang(target, source, env):
             """Copy source to target, rewriting the shebang"""
             # Currently just use this python
             usepython = utils.whichPython()
-            for targ, src in zip(target, source):
+            for targ, src in zip(target, source, strict=True):
                 with open(str(src)) as srcfd:
                     with open(str(targ), "w") as outfd:
                         first_line = srcfd.readline()
@@ -455,7 +455,7 @@ def python(module=None, src=None, extra=(), libs="main python"):
         return result
 
     @staticmethod
-    def pybind11(nameList=[], libs="main python", extraSrc=None, addUnderscore=True):
+    def pybind11(nameList=(), libs="main python", extraSrc=None, addUnderscore=True):
         """Convenience function to replace standard ``python/*/SConscript``
         boilerplate.
 
@@ -467,7 +467,7 @@ def pybind11(nameList=[], libs="main python", extraSrc=None, addUnderscore=True)
 
         Parameters
         ----------
-        nameList : `list`, optional
+        nameList : `collections.abc.Sequence`, optional
             Sequence of pybind11 modules to be built (does not include the
             file extensions).
         libs : `str` or `list`, optional
@@ -642,7 +642,7 @@ def tests(
         if swigSrc is None:
             swigSrc = {}
         allSwigSrc = set()
-        for name, node in zip(swigNameList, swigFileList):
+        for name, node in zip(swigNameList, swigFileList, strict=True):
             src = swigSrc.setdefault(name, [])
             allSwigSrc.update(str(element) for element in src)
             src.append(node)
@@ -760,7 +760,7 @@ def examples(ccList=None, swigNameList=None, swigSrc=None):
         if swigSrc is None:
             swigSrc = {}
         allSwigSrc = set()
-        for name, node in zip(swigNameList, swigFileList):
+        for name, node in zip(swigNameList, swigFileList, strict=True):
             src = swigSrc.setdefault(name, [])
             allSwigSrc.update(str(element) for element in src)
             src.append(node)

python/lsst/sconsUtils/scripts.py

@@ -72,7 +72,7 @@ def _initOptions():
         action="store_true",
         default=False,
         help="Filter out a class of warnings deemed irrelevant",
-    ),
+    )
     SCons.Script.AddOption(
         "--force",
         dest="force",
@@ -579,13 +579,13 @@ def ClassifyCc(context):
     #
     if re.search(r"^(Linux|Linux64)$", env["eupsFlavor"]) and "LD_LIBRARY_PATH" in os.environ:
         env.Append(LINKFLAGS=["-Wl,-rpath-link"])
-        env.Append(LINKFLAGS=[f'-Wl,{os.environ["LD_LIBRARY_PATH"]}'])
+        env.Append(LINKFLAGS=[f"-Wl,{os.environ['LD_LIBRARY_PATH']}"])
     #
     # Set the optimization level.
     #
     if env["opt"]:
         env["CCFLAGS"] = [o for o in env["CCFLAGS"] if not re.search(r"^-O(\d|s|g|fast)$", o)]
-        env.MergeFlags(f'-O{env["opt"]}')
+        env.MergeFlags(f"-O{env['opt']}")
     #
     # Set compiler-specific warning flags.
     #

python/lsst/sconsUtils/state.py

@@ -19,15 +19,15 @@
 # the extra .linkinfo files when calling scons -c
 def CUDANVCCStaticObjectEmitter(target, source, env):
     tgt, src = SCons.Defaults.StaticObjectEmitter(target, source, env)
-    for file in src:
+    for _ in src:
         lifile = os.path.splitext(src[0].rstr())[0] + ".linkinfo"  # noqa F841
         # tgt.append(lifile)
     return tgt, src
 
 
 def CUDANVCCSharedObjectEmitter(target, source, env):
     tgt, src = SCons.Defaults.SharedObjectEmitter(target, source, env)
-    for file in src:
+    for _ in src:
         lifile = os.path.splitext(src[0].rstr())[0] + ".linkinfo"  # noqa F841
         # tgt.append(lifile)
     return tgt, src

python/lsst/sconsUtils/tools/cuda.py

@@ -5,13 +5,13 @@
 __all__ = (
     "Log",
     "_has_OSX_SIP",
+    "get_conda_prefix",
+    "libraryLoaderEnvironment",
     "libraryPathPassThrough",
-    "whichPython",
+    "memberOf",
     "needShebangRewrite",
-    "libraryLoaderEnvironment",
     "runExternal",
-    "memberOf",
-    "get_conda_prefix",
+    "whichPython",
 )
 
 import os