Merge pull request #16 from lsst/tickets/DM-39857

DM-39857: Clean up pytest-flake8 and add ruff configuration

.github/workflows/build.yaml

@@ -40,7 +40,7 @@ jobs:
       - name: Install pytest packages
         run: |
           pip install \
-            pytest pytest-flake8 pytest-xdist pytest-openfiles pytest-cov "flake8<5"
+            pytest pytest-xdist pytest-openfiles pytest-cov
 
       - name: List installed packages
         run: |

.github/workflows/build_docs.yaml

@@ -22,6 +22,9 @@ jobs:
           cache: "pip"
           cache-dependency-path: "setup.cfg"
 
+      - name: Install graphviz
+        run: sudo apt-get install graphviz
+
       - name: Install sqlite
         run: sudo apt-get install sqlite libyaml-dev
 
@@ -38,8 +41,8 @@ jobs:
         run: pip install -v .
 
       - name: Install documenteer
-        run: pip install 'documenteer[pipelines]<0.7'
+        run: pip install 'documenteer[pipelines]<0.9'
 
       - name: Build documentation
         working-directory: ./doc
-        run: package-docs build
+        run: package-docs build -W

doc/conf.py

@@ -5,14 +5,13 @@
 https://developer.lsst.io/stack/building-single-package-docs.html
 """
 
-from documenteer.sphinxconfig.stackconf import build_package_configs  # noqa: F401
-
-master_doc = "index"
-extensions = ["sphinx_automodapi.automodapi"]
+from documenteer.conf.pipelinespkg import *  # noqa: F403, import *
 
 project = "ctrl_bps_parsl"
-html_theme_options = dict(logotext=project)
+html_theme_options["logotext"] = project  # noqa: F405, unknown name
 html_title = project
 html_short_title = project
 exclude_patterns = ["changes/*"]
 numpydoc_show_class_members = False
+
+intersphinx_mapping["lsst"] = ("https://pipelines.lsst.io/v/daily/", None)  # noqa

pyproject.toml

@@ -31,9 +31,6 @@ dynamic = ["version"]
 [project.optional-dependencies]
 test = [
     "pytest >= 3.2",
-    "flake8 >= 3.7.5",
-    "flake8 < 5",
-    "pytest-flake8 >= 1.0.4",
     "pytest-openfiles >= 0.5.0"
 ]
 
@@ -99,9 +96,8 @@ line_length = 110
 [tool.lsst_versions]
 write_to = "python/lsst/ctrl/bps/parsl/version.py"
 
-#[tool.pytest.ini_options]
-#addopts = "--flake8"
-#flake8-ignore = ["W503", "E203", "N802", "N803", "N806", "N812", "N815", "N816"]
+[tool.pytest.ini_options]
+# Retain config entry to prevent pytest from looking in parent directory.
 
 [tool.pydocstyle]
 convention = "numpy"
@@ -111,4 +107,44 @@ convention = "numpy"
 # Docstring at the very first line is not required
 # D200, D205 and D400 all complain if the first sentence of the docstring does
 # not fit on one line.
-add-ignore = ["D107", "D105", "D102", "D100", "D200", "D205", "D400"]
+add-ignore = ["D107", "D105", "D102", "D100", "D200", "D205", "D400", "D104"]
+
+[tool.ruff]
+exclude = [
+    "__init__.py",
+]
+ignore = [
+    "N802",
+    "N803",
+    "N806",
+    "N812",
+    "N815",
+    "N816",
+    "N999",
+    "D107",
+    "D105",
+    "D102",
+    "D104",
+    "D100",
+    "D200",
+    "D205",
+    "D400",
+]
+line-length = 110
+select = [
+    "E",  # pycodestyle
+    "F",  # pyflakes
+    "N",  # pep8-naming
+    "W",  # pycodestyle
+    "D",  # pydocstyle
+]
+target-version = "py310"
+extend-select = [
+    "RUF100", # Warn about unused noqa
+]
+
+[tool.ruff.pycodestyle]
+max-doc-length = 79
+
+[tool.ruff.pydocstyle]
+convention = "numpy"

python/conftest.py

@@ -4,7 +4,6 @@
 # The following code is from https://stackoverflow.com/a/72366347/834250
 
 import pathlib
-from typing import Optional
 
 import _pytest.pathlib
 
@@ -16,7 +15,8 @@
 
 
 # patched method
-def resolve_package_path(path: pathlib.Path) -> Optional[pathlib.Path]:
+def resolve_package_path(path: pathlib.Path) -> pathlib.Path | None:
+    """Resolve the supplied path."""
     # call original lookup
     result = resolve_pkg_path_orig(path)
     if result is None:

python/lsst/ctrl/bps/parsl/configuration.py

@@ -1,6 +1,6 @@
 import logging
 import os
-from typing import Any, Dict, Literal, Optional, Type, TypeVar, overload
+from typing import Any, Literal, TypeVar, overload
 
 from lsst.ctrl.bps import BpsConfig
 
@@ -20,7 +20,7 @@
 def get_bps_config_value(
     config: BpsConfig,
     key: str,
-    dataType: Type[T],
+    dataType: type[T],
     default: T,
 ) -> T:
     ...
@@ -31,8 +31,8 @@ def get_bps_config_value(
 def get_bps_config_value(
     config: BpsConfig,
     key: str,
-    dataType: Type[T],
-    default: Optional[T] = None,
+    dataType: type[T],
+    default: T | None = None,
     *,
     required: Literal[True],
 ) -> T:
@@ -44,20 +44,20 @@ def get_bps_config_value(
 def get_bps_config_value(
     config: BpsConfig,
     key: str,
-    dataType: Type[T],
-    default: Optional[T] = None,
-) -> Optional[T]:
+    dataType: type[T],
+    default: T | None = None,
+) -> T | None:
     ...
 
 
 def get_bps_config_value(
     config: BpsConfig,
     key: str,
-    dataType: Type[T],
-    default: Optional[T] = None,
+    dataType: type[T],
+    default: T | None = None,
     *,
     required: bool = False,
-) -> Optional[T]:
+) -> T | None:
     """Get a value from the BPS configuration
 
     I find this more useful than ``BpsConfig.__getitem__`` or
@@ -92,7 +92,7 @@ def get_bps_config_value(
     RuntimeError
         If the value is not set or is of the wrong type.
     """
-    options: Dict[str, Any] = dict(expandEnvVars=True, replaceVars=True, required=required)
+    options: dict[str, Any] = dict(expandEnvVars=True, replaceVars=True, required=required)
     if default is not None:
         options["default"] = default
     found, value = config.search(key, options)

python/lsst/ctrl/bps/parsl/job.py

@@ -1,9 +1,10 @@
 import os
 import re
 import subprocess
+from collections.abc import Sequence
 from functools import partial
 from textwrap import dedent
-from typing import Any, Dict, List, Optional, Sequence
+from typing import Any
 
 from lsst.ctrl.bps import BpsConfig, GenericWorkflow, GenericWorkflowJob
 from parsl.app.bash import BashApp
@@ -20,9 +21,9 @@
 def run_command(
     command_line: str,
     inputs: Sequence[Future] = (),
-    stdout: Optional[str] = None,
-    stderr: Optional[str] = None,
-    parsl_resource_specification: Optional[Dict[str, Any]] = None,
+    stdout: str | None = None,
+    stderr: str | None = None,
+    parsl_resource_specification: dict[str, Any] | None = None,
 ) -> str:
     """Run a command
 
@@ -49,7 +50,7 @@ def run_command(
     return command_line
 
 
-def get_file_paths(workflow: GenericWorkflow, name: str) -> Dict[str, str]:
+def get_file_paths(workflow: GenericWorkflow, name: str) -> dict[str, str]:
     """Extract file paths for a job
 
     Parameters
@@ -84,7 +85,7 @@ def __init__(
         self,
         generic: GenericWorkflowJob,
         config: BpsConfig,
-        file_paths: Dict[str, str],
+        file_paths: dict[str, str],
     ):
         self.generic = generic
         self.name = generic.name
@@ -167,7 +168,7 @@ def evaluate_command_line(self, command: str) -> str:
         command = re.sub(_file_regex, lambda match: self.file_paths[match.group(1)], command)  # Files
         return command
 
-    def get_resources(self) -> Dict[str, Any]:
+    def get_resources(self) -> dict[str, Any]:
         """Return what resources are required for executing this job"""
         resources = {}
         for bps_name, parsl_name, scale in (
@@ -185,10 +186,10 @@ def get_resources(self) -> Dict[str, Any]:
     def get_future(
         self,
         app: BashApp,
-        inputs: List[Future],
-        command_prefix: Optional[str] = None,
+        inputs: list[Future],
+        command_prefix: str | None = None,
         add_resources: bool = False,
-    ) -> Optional[Future]:
+    ) -> Future | None:
         """Get the parsl app future for the job
 
         This effectively queues the job for execution by a worker, subject to

python/lsst/ctrl/bps/parsl/service.py

@@ -1,5 +1,3 @@
-from typing import Tuple
-
 from lsst.ctrl.bps import BaseWmsService, BaseWmsWorkflow, BpsConfig, GenericWorkflow
 
 from .workflow import ParslWorkflow
@@ -47,7 +45,7 @@ def submit(self, workflow: BaseWmsWorkflow):
         workflow.start()
         workflow.run()
 
-    def restart(self, out_prefix: str) -> Tuple[str, str, str]:
+    def restart(self, out_prefix: str) -> tuple[str, str, str]:
         """Restart a workflow from the point of failure.
 
         Parameters

python/lsst/ctrl/bps/parsl/site.py

@@ -1,6 +1,6 @@
 from abc import ABC, abstractmethod
 from types import ModuleType
-from typing import TYPE_CHECKING, List, Optional
+from typing import TYPE_CHECKING
 
 from lsst.ctrl.bps import BpsConfig
 from lsst.utils import doImport
@@ -86,7 +86,7 @@ def from_config(cls, config: BpsConfig) -> "SiteConfig":
         return site_config(config)
 
     @abstractmethod
-    def get_executors(self) -> List[ParslExecutor]:
+    def get_executors(self) -> list[ParslExecutor]:
         """Get a list of executors to be used in processing
 
         Each executor should have a unique ``label``.
@@ -142,7 +142,7 @@ def get_command_prefix(self) -> str:
             prefix += "\n" + export_environment()
         return prefix
 
-    def get_monitor(self) -> Optional[MonitoringHub]:
+    def get_monitor(self) -> MonitoringHub | None:
         """Get parsl monitor
 
         The parsl monitor provides a database that tracks the progress of the

python/lsst/ctrl/bps/parsl/sites/local.py

@@ -1,4 +1,4 @@
-from typing import TYPE_CHECKING, List
+from typing import TYPE_CHECKING
 
 from parsl.executors import HighThroughputExecutor
 from parsl.executors.base import ParslExecutor
@@ -20,7 +20,7 @@ class Local(SiteConfig):
     ``site.<computeSite>.cores`` (`int`).
     """
 
-    def get_executors(self) -> List[ParslExecutor]:
+    def get_executors(self) -> list[ParslExecutor]:
         """Get a list of executors to be used in processing
 
         Each executor should have a unique ``label``.

python/lsst/ctrl/bps/parsl/sites/nersc.py

@@ -1,11 +1,11 @@
-from typing import Any, Dict, List, Optional
+from typing import Any
 
 from parsl.executors.base import ParslExecutor
 from parsl.launchers import SrunLauncher
 
 from .slurm import TripleSlurm
 
-Kwargs = Dict[str, Any]
+Kwargs = dict[str, Any]
 
 __all__ = ("CoriKnl",)
 
@@ -39,25 +39,25 @@ class CoriKnl(TripleSlurm):
 
     def get_executors(
         self,
-        small_options: Optional[Kwargs] = None,
-        medium_options: Optional[Kwargs] = None,
-        large_options: Optional[Kwargs] = None,
+        small_options: Kwargs | None = None,
+        medium_options: Kwargs | None = None,
+        large_options: Kwargs | None = None,
         **common_options,
-    ) -> List[ParslExecutor]:
-        """Get a list of executors to be used in processing
+    ) -> list[ParslExecutor]:
+        """Get a list of executors to be used in processing.
 
         We create three executors, with different walltime and memory per
         worker.
 
         Parameters
         ----------
-        small_options : kwargs
+        small_options : `dict`
             Options for ``make_executor`` for small executor.
-        medium_options : kwargs
+        medium_options : `dict`
             Options for ``make_executor`` for medium executor.
-        large_options : kwargs
+        large_options : `dict`
             Options for ``make_executor`` for large executor.
-        **common_options
+        **common_options : Any
             Common options for ``make_executor`` for each of the executors.
         """
         scheduler_options = "\n".join(

python/lsst/ctrl/bps/parsl/sites/princeton.py

@@ -1,4 +1,4 @@
-from typing import TYPE_CHECKING, List
+from typing import TYPE_CHECKING
 
 from parsl.addresses import address_by_interface
 from parsl.executors.base import ParslExecutor
@@ -46,7 +46,7 @@ class Tiger(Slurm):
     everything has completed.
     """
 
-    def get_executors(self) -> List[ParslExecutor]:
+    def get_executors(self) -> list[ParslExecutor]:
         """Get a list of executors to be used in processing.
 
         Each executor should have a unique ``label``.

python/lsst/ctrl/bps/parsl/sites/slac.py

@@ -1,4 +1,4 @@
-from typing import TYPE_CHECKING, List
+from typing import TYPE_CHECKING
 
 from parsl.executors.base import ParslExecutor
 from parsl.launchers import SrunLauncher
@@ -29,7 +29,7 @@ class Sdf(Slurm):
       ``True``.
     """
 
-    def get_executors(self) -> List[ParslExecutor]:
+    def get_executors(self) -> list[ParslExecutor]:
         """Get a list of executors to be used in processing
 
         Each executor should have a unique ``label``.