Switch linting/formatting to ruff (#738)

* Move isort,black,flake8 to ruff
* Update formatting to latest black style before switch
* Lint fixes, both manually and automatic (including unsafe fixes, manually checked)

.azure-pipelines/azure-pipelines.yml

@@ -35,10 +35,10 @@ stages:
 
       - bash: |
           set -eux
-          pip install flake8
+          pip install ruff
           cd repository
-          python .azure-pipelines/flake8-validation.py
-        displayName: Flake8 validation
+          python .azure-pipelines/lint-validation.py
+        displayName: Ruff validation
 
       # Set up constants for further build steps
       - bash: |

.azure-pipelines/bootstrap.py

@@ -122,9 +122,7 @@ def install_micromamba(python):
 There was a failure in constructing the conda environment.
 Attempt {retry} of 5 will start {retry} minute(s) from {t}.
 *******************************************************************************
-""".format(
-                    retry=retry, t=time.asctime()
-                )
+""".format(retry=retry, t=time.asctime())
             )
             time.sleep(retry * 60)
         else:

.azure-pipelines/flake8-validation.py → .azure-pipelines/lint-validation.py

@@ -5,9 +5,10 @@
 
 failures = 0
 try:
-    flake8 = subprocess.run(
+    process = subprocess.run(
         [
-            "flake8",
+            "ruff",
+            "check",
             "--exit-zero",
         ],
         capture_output=True,
@@ -17,14 +18,14 @@
     )
 except (subprocess.CalledProcessError, subprocess.TimeoutExpired) as e:
     print(
-        "##vso[task.logissue type=error;]flake8 validation failed with",
+        "##vso[task.logissue type=error;]Ruff validation failed with",
         str(e.__class__.__name__),
     )
     print(e.stdout)
     print(e.stderr)
-    print("##vso[task.complete result=Failed;]flake8 validation failed")
+    print("##vso[task.complete result=Failed;]Ruff validation failed")
     exit()
-for line in flake8.stdout.split("\n"):
+for line in process.stdout.split("\n"):
     if ":" not in line:
         continue
     filename, lineno, column, error = line.split(":", maxsplit=3)
@@ -37,5 +38,5 @@
     )
 
 if failures:
-    print(f"##vso[task.logissue type=warning]Found {failures} flake8 violation(s)")
-    print(f"##vso[task.complete result=Failed;]Found {failures} flake8 violation(s)")
+    print(f"##vso[task.logissue type=warning]Found {failures} Ruff violation(s)")
+    print(f"##vso[task.complete result=Failed;]Found {failures} Ruff violation(s)")

.pre-commit-config.yaml

@@ -1,7 +1,7 @@
 repos:
 # Syntax validation and some basic sanity checks
 - repo: https://github.com/pre-commit/pre-commit-hooks
-  rev: v4.4.0
+  rev: v4.6.0
   hooks:
   - id: check-merge-conflict
   - id: check-ast
@@ -14,41 +14,17 @@ repos:
   - id: no-commit-to-branch
     name: "Don't commit to 'main' directly"
 
-# Automatically sort imports
-- repo: https://github.com/PyCQA/isort
-  rev: 5.12.0
+- repo: https://github.com/charliermarsh/ruff-pre-commit
+  rev: v0.4.8
   hooks:
-  - id: isort
-    args: [
-           '-a', 'from __future__ import annotations',        # 3.7-3.11
-           '--rm', 'from __future__ import absolute_import',  # -3.0
-           '--rm', 'from __future__ import division',         # -3.0
-           '--rm', 'from __future__ import generator_stop',   # -3.7
-           '--rm', 'from __future__ import generators',       # -2.3
-           '--rm', 'from __future__ import nested_scopes',    # -2.2
-           '--rm', 'from __future__ import print_function',   # -3.0
-           '--rm', 'from __future__ import unicode_literals', # -3.0
-           '--rm', 'from __future__ import with_statement',   # -2.6
-          ]
-
-# Automatic source code formatting
-- repo: https://github.com/psf/black
-  rev: 22.12.0
-  hooks:
-  - id: black
-    args: [--safe, --quiet]
-    files: \.pyi?$|SConscript$|^libtbx_config$
-    types: [file]
+    - id: ruff
+      args: [--fix, --exit-non-zero-on-fix, --show-fixes]
+    - id: ruff-format
+      files: \.pyi?$|SConscript$|^libtbx_config$
+      types: [file]
 
 - repo: https://github.com/pre-commit/mirrors-clang-format
   rev: v14.0.6
   hooks:
   - id: clang-format
     files: \.c(c|pp|xx)?$|\.h(pp)?$
-
-# Linting
-- repo: https://github.com/PyCQA/flake8
-  rev: 6.0.0
-  hooks:
-  - id: flake8
-    additional_dependencies: ['flake8-comprehensions==3.8.0']

CONTRIBUTING.md

@@ -96,7 +96,7 @@ message that the code in question is special and care should be taken.
 - **Please install the pre-commit hooks**. (`libtbx.precommit install` if using
   the libtbx ecosystem). These use the [pre-commit] library and ensure that
   various sanity checks are run before commit, including formatting, syntax
-  compatibility, basic flake8 checks, lack of conflict markers and file size
+  compatibility, basic Ruff checks, lack of conflict markers and file size
   limits. Basically, most of the essential rules will be checked automatically
   by this.
 - **We format python code with [black]**. This means that while writing code
@@ -106,13 +106,13 @@ message that the code in question is special and care should be taken.
   you can't, the whole codebase is auto-cleaned once a week. Most IDEs and
   editors have support for running formatters like black frequently or
   automatically.
-- **Avoid introducing new pre-commit flake8 warnings** - if you feel that it's
+- **Avoid introducing new pre-commit Ruff warnings** - if you feel that it's
   appropriate to violate a warning, mark it up explicitly with a [noqa]
   comment. Probably the most common cause of this are "F401 - module imported
   or unused", which happens when importing packages to collect into a single
   namespace for other imports (though declaring `__all__` avoids this issue).
   The pre-commit hooks will pick up the most important of these, but please try
-  to resolve any other valid warnings shown with a normal run of flake8. The
+  to resolve any other valid warnings shown with a normal run of Ruff. The
   configuration in the repository turns off any that disagree with black's
   interpretation of the rules or standard practice in our repositories.
 - **We format C++ code with [clang-format]**. We use a configuration for style
@@ -125,7 +125,7 @@ message that the code in question is special and care should be taken.
 [pre-commit]: https://github.com/pre-commit/pre-commit
 [black]: https://github.com/python/black
 [clang-format]: https://clang.llvm.org/docs/ClangFormat.html
-[noqa]: http://flake8.pycqa.org/en/3.7.7/user/violations.html#in-line-ignoring-errors
+[noqa]: http://Ruff.pycqa.org/en/3.7.7/user/violations.html#in-line-ignoring-errors
 [PEP8]: https://www.python.org/dev/peps/pep-0008
 [Google-style]: https://sphinxcontrib-napoleon.readthedocs.io/en/latest/example_google.html
 [Zen of Python]: https://www.python.org/dev/peps/pep-0020/#the-zen-of-python

SConscript

@@ -119,7 +119,11 @@ if not env_etc.no_boost_python and hasattr(env_etc, "boost_adaptbx_include"):
     env = env_no_includes_boost_python_ext.Clone()
 
     # Don't surface warnings from system or cctbx_project headers
-    system_includes = [x for x in env_etc.conda_cpppath if x] if libtbx.env.build_options.use_conda else []
+    system_includes = (
+        [x for x in env_etc.conda_cpppath if x]
+        if libtbx.env.build_options.use_conda
+        else []
+    )
     system_includes.append(str(Path(env_etc.scitbx_dist).parent))
     env.Append(CXXFLAGS=[f"-isystem{x}" for x in system_includes])
     env.Append(SHCXXFLAGS=[f"-isystem{x}" for x in system_includes])

newsfragments/738.misc

@@ -0,0 +1 @@
+Switch linting/formatting to ruff.

pyproject.toml

@@ -1,18 +1,69 @@
 [tool.black]
-include='\.pyi?$|/SConscript$|/libtbx_config$'
+include = '\.pyi?$|/SConscript$|/libtbx_config$'
 
 [tool.towncrier]
 package = "dxtbx"
 package_dir = ".."
 filename = "CHANGELOG.rst"
 issue_format = "`#{issue} <https://github.com/cctbx/dxtbx/issues/{issue}>`_"
 
-[tool.isort]
-sections="FUTURE,STDLIB,THIRDPARTY,CCTBX,DIALS,FIRSTPARTY,LOCALFOLDER"
-known_firstparty="dxtbx_*,dxtbx"
-known_cctbx="boost,boost_adaptbx,cbflib_adaptbx,cctbx,chiltbx,clipper_adaptbx,cma_es,cootbx,crys3d,cudatbx,fable,fast_linalg,fftw3tbx,gltbx,iota,iotbx,libtbx,mmtbx,omptbx,prime,rstbx,scitbx,simtbx,smtbx,spotfinder,tbxx,ucif,wxtbx,xfel"
-known_dials="dials"
-profile="black"
+
+[tool.ruff.lint]
+select = ["E", "F", "W", "C4", "I"]
+unfixable = ["F841"]
+# E501 line too long (handled by formatter)
+# E741 Ambiguous variable name (We have lots of meaningful I, L, l)
+ignore = ["E501", "E741"]
+
+[tool.ruff.lint.per-file-ignores]
+"installer/**.py" = ["I"]
+"**/__init__.py" = ["F401"]
+
+[tool.ruff.lint.isort]
+known-first-party = ["dxtbx_*", "dxtbx"]
+required-imports = ["from __future__ import annotations"]
+section-order = [
+    "future",
+    "standard-library",
+    "third-party",
+    "cctbx",
+    "first-party",
+    "local-folder",
+]
+
+[tool.ruff.lint.isort.sections]
+"cctbx" = [
+    "boost",
+    "boost_adaptbx",
+    "cbflib_adaptbx",
+    "cctbx",
+    "chiltbx",
+    "clipper_adaptbx",
+    "cma_es",
+    "cootbx",
+    "crys3d",
+    "cudatbx",
+    "fable",
+    "fast_linalg",
+    "fftw3tbx",
+    "gltbx",
+    "iota",
+    "iotbx",
+    "libtbx",
+    "mmtbx",
+    "omptbx",
+    "prime",
+    "rstbx",
+    "scitbx",
+    "serialtbx",
+    "simtbx",
+    "smtbx",
+    "spotfinder",
+    "tbxx",
+    "ucif",
+    "wxtbx",
+    "xfel",
+]
 
 [tool.mypy]
 no_implicit_optional = true

setup.cfg

@@ -11,20 +11,3 @@ classifiers =
     Programming Language :: Python :: 3.9
     Programming Language :: Python :: 3.10
     Programming Language :: Python :: 3.11
-
-[flake8]
-# Black disagrees with flake8 on a few points. Ignore those.
-ignore = E203, E266, E501, W503
-# E203 whitespace before ':'
-# E266 too many leading '#' for block comment
-# E501 line too long
-# W503 line break before binary operator
-
-max-line-length = 88
-
-select =
-    E401,E711,E712,E713,E714,E721,E722,E901,
-    F401,F402,F403,F405,F541,F631,F632,F633,F811,F812,F821,F822,F841,F901,
-    W191,W291,W292,W293,W602,W603,W604,W605,W606,
-    # flake8-comprehensions, https://github.com/adamchainz/flake8-comprehensions
-    C4,

src/dxtbx/command_line/detector_superpose.py

@@ -3,16 +3,15 @@
 import math
 import sys
 
-from serialtbx.detector import iterate_detector_at_level
+import dials.util
+from dials.util.options import OptionParser
 
 from libtbx.phil import parse
 from libtbx.test_utils import approx_equal
 from scitbx.array_family import flex
 from scitbx.math.superpose import least_squares_fit
 from scitbx.matrix import col
-
-import dials.util
-from dials.util.options import OptionParser
+from serialtbx.detector import iterate_detector_at_level
 
 import dxtbx.util
 from dxtbx.model.experiment_list import ExperimentListFactory
@@ -92,10 +91,9 @@ def run(args=None):
             "Reference detector must be at least %d panels long given the panel list"
             % (max_p_id + 1)
         )
-        assert max_p_id < len(
-            moving
-        ), "Moving detector must be at least %d panels long given the panel list" % (
-            max_p_id + 1
+        assert max_p_id < len(moving), (
+            "Moving detector must be at least %d panels long given the panel list"
+            % (max_p_id + 1)
         )
         panel_ids = params.panel_list
 

src/dxtbx/command_line/image2pickle.py

@@ -12,10 +12,10 @@
 import sys
 
 import numpy as np
-import serialtbx.detector.cspad
-import serialtbx.util
 
 import libtbx.option_parser
+import serialtbx.detector.cspad
+import serialtbx.util
 from libtbx import easy_pickle
 from libtbx.utils import Usage
 from scitbx.array_family import flex

src/dxtbx/command_line/plot_detector_models.py

@@ -125,7 +125,6 @@ def plot_group(
 
 
 def plot_image_plane_projection(detector, color, ax, panel_numbers=True):
-
     origin_2d, fast_2d, slow_2d = get_detector_projection_2d_axes(detector)
 
     for panel, origin, fast, slow in zip(detector, origin_2d, fast_2d, slow_2d):
@@ -182,7 +181,6 @@ def run(args=None):
     min_z = max_z = None
     ax = None
     for file_name, color in zip(files, colors):
-
         # read the data and get the detector models
         try:
             experiments = ExperimentListFactory.from_json_file(

src/dxtbx/command_line/read_sequence.py

@@ -12,7 +12,6 @@
 
 
 def read_sequence(images: list[str]):
-
     sequences = ImageSetFactory.new(images)
 
     for sequence in sequences:

src/dxtbx/data/beamline_defs/PILATUS_300K_S_N_3_0104_Diamond.py

@@ -45,9 +45,9 @@ def _at_I19(self, mmcif=False):
         b[lookup("df.rad.mono")] = "Silicon 111"
         b[lookup("df.src.details")] = "Nowell et al. (2012)"
         b[lookup("df.src.type")] = "Diamond Light Source Beamline I19-2"
-        b[
-            lookup("references")
-        ] = "Nowell, H. et al. (2012) J. Synchrotron Rad. 19, 435-441."
+        b[lookup("references")] = (
+            "Nowell, H. et al. (2012) J. Synchrotron Rad. 19, 435-441."
+        )
         b[lookup("sw.collection")] = "GDA - generic data acquisition software"
 
         return b

src/dxtbx/dxtbx_imageset_ext.pyi

@@ -21,14 +21,12 @@ class ExternalLookup:
     def pedestal(self) -> Any: ...
 
 class ExternalLookupItemBool:
-
     data: Any
     filename: Any
     def __init__(self, *args, **kwargs) -> None: ...
     def __reduce__(self) -> Any: ...
 
 class ExternalLookupItemDouble:
-
     data: Any
     filename: Any
     def __init__(self, *args, **kwargs) -> None: ...

src/dxtbx/dxtbx_model_ext.pyi

@@ -22,8 +22,10 @@ from scitbx.array_family import shared as flex_shared
 # Attempt to use the stub typing for flex-inheritance
 from scitbx.array_family.flex import FlexPlain
 
-from dxtbx_model_ext import Probe  # type: ignore
-from dxtbx_model_ext import ExperimentType
+from dxtbx_model_ext import (
+    ExperimentType,
+    Probe,  # type: ignore
+)
 
 # TypeVar for the set of Experiment models that can be joint-accepted
 # - profile, imageset and scalingmodel are handled as 'object'

src/dxtbx/example/resolution_corners.py

@@ -1,6 +1,5 @@
 """Print out the resolution (two-theta) of the corners of the detector"""
 
-
 from __future__ import annotations
 
 import math