Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch linting/formatting to ruff #738

Merged
merged 13 commits into from
Jun 7, 2024
6 changes: 3 additions & 3 deletions .azure-pipelines/azure-pipelines.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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: |
Expand Down
4 changes: 1 addition & 3 deletions .azure-pipelines/bootstrap.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,10 @@

failures = 0
try:
flake8 = subprocess.run(
process = subprocess.run(
[
"flake8",
"ruff",
"check",
"--exit-zero",
],
capture_output=True,
Expand All @@ -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)
Expand All @@ -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)")
40 changes: 8 additions & 32 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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']
8 changes: 4 additions & 4 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
6 changes: 5 additions & 1 deletion SConscript
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand Down
1 change: 1 addition & 0 deletions newsfragments/738.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Switch linting/formatting to ruff.
65 changes: 58 additions & 7 deletions pyproject.toml
Original file line number Diff line number Diff line change
@@ -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
17 changes: 0 additions & 17 deletions setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -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,
14 changes: 6 additions & 8 deletions src/dxtbx/command_line/detector_superpose.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Check warning on line 7 in src/dxtbx/command_line/detector_superpose.py

View check run for this annotation

Codecov / codecov/patch

src/dxtbx/command_line/detector_superpose.py#L6-L7

Added lines #L6 - L7 were not covered by tests

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

Check warning on line 14 in src/dxtbx/command_line/detector_superpose.py

View check run for this annotation

Codecov / codecov/patch

src/dxtbx/command_line/detector_superpose.py#L14

Added line #L14 was not covered by tests

import dxtbx.util
from dxtbx.model.experiment_list import ExperimentListFactory
Expand Down Expand Up @@ -92,10 +91,9 @@
"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), (

Check warning on line 94 in src/dxtbx/command_line/detector_superpose.py

View check run for this annotation

Codecov / codecov/patch

src/dxtbx/command_line/detector_superpose.py#L94

Added line #L94 was not covered by tests
"Moving detector must be at least %d panels long given the panel list"
% (max_p_id + 1)
)
panel_ids = params.panel_list

Expand Down
4 changes: 2 additions & 2 deletions src/dxtbx/command_line/image2pickle.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Check warning on line 18 in src/dxtbx/command_line/image2pickle.py

View check run for this annotation

Codecov / codecov/patch

src/dxtbx/command_line/image2pickle.py#L17-L18

Added lines #L17 - L18 were not covered by tests
from libtbx import easy_pickle
from libtbx.utils import Usage
from scitbx.array_family import flex
Expand Down
2 changes: 0 additions & 2 deletions src/dxtbx/command_line/plot_detector_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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(
Expand Down
1 change: 0 additions & 1 deletion src/dxtbx/command_line/read_sequence.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@


def read_sequence(images: list[str]):

sequences = ImageSetFactory.new(images)

for sequence in sequences:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,9 @@
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")] = (

Check warning on line 48 in src/dxtbx/data/beamline_defs/PILATUS_300K_S_N_3_0104_Diamond.py

View check run for this annotation

Codecov / codecov/patch

src/dxtbx/data/beamline_defs/PILATUS_300K_S_N_3_0104_Diamond.py#L48

Added line #L48 was not covered by tests
"Nowell, H. et al. (2012) J. Synchrotron Rad. 19, 435-441."
)
b[lookup("sw.collection")] = "GDA - generic data acquisition software"

return b
2 changes: 0 additions & 2 deletions src/dxtbx/dxtbx_imageset_ext.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -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: ...
Expand Down
6 changes: 4 additions & 2 deletions src/dxtbx/dxtbx_model_ext.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
1 change: 0 additions & 1 deletion src/dxtbx/example/resolution_corners.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
"""Print out the resolution (two-theta) of the corners of the detector"""


from __future__ import annotations

import math
Expand Down
Loading
Loading