misc: Ruff #2771

JDBetteridge · 2025-10-16T23:08:53Z

I'm trying to automate detection a lot of the "nitpick" code review comments and providing a route to automatically fixing the issues apriori.

What I have added to CI is equivalent to locally running:

isort --check-only --sort-reexports --multi-line VERTICAL_GRID_GROUPED --line-length 90 --force-alphabetical-sort-within-sections --project devito,examples .
ruff check --preview  --select E,W,F,B,UP,SIM,I,RUF022 --ignore F403,E226,E731,E275,F405,E722,E741,W605 --line-length=90 --output-format concise

Most command line arguments are included in the pyproject.toml and checking out this branch you can just run

isort --check-only .
ruff check --output-format concise

Fixing most linting issues is as simple as

isort .
ruff check --fix

if you trust the auto-formatter!

This PR also extends linting to Dockerfiles and Github actions too.

An excerpt from CONTRIBUTING.md:

Coding guidelines

Some coding rules are "enforced" (and automatically checked by our Continuous Integration systems), some are "strongly recommended", others are "optional" but welcome.

We enforce PEP8, with a few exceptions, listed here

We enforce a maximum line length of 90 characters.

We enforce indentation via 4 spaces.

We suggest to use flake8 to check the above points locally, before filing a Pull Request.

We strongly recommend to document any new module, class, routine, ... with NumPy-like docstrings ("numpydoc").

We strongly recommend imports to be at the top of a module, logically grouped and, within each group, to be alphabetically ordered. As an example, condider our init.py: the first group is imports from the standard library; then imports from third-party dependencies; finally, imports from devito modules.

We strongly recommend to follow standard Python coding guidelines:

Use camel caps for class names, e.g. class FooBar.

Method names must start with a small letter; use underscores to separate words, e.g. def _my_meth_....

Private class attributes and methods must start with an underscore.

Variable names should be explicative (Devito prefers "long and clear" over "short but unclear").

Comment your code, and do not be afraid of being verbose. The first letter must be capitalized. Do not use punctuation (unless the comment consists of multiple sentences).

We like that blank lines are used to logically split blocks of code implementing different (possibly sequential) tasks.

.github/workflows/lint.yml

codecov · 2025-10-16T23:37:13Z

Codecov Report

❌ Patch coverage is 77.51736% with 259 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.97%. Comparing base (3e6c852) to head (8d1fa60).

Files with missing lines	Patch %	Lines
devito/mpi/routines.py	13.55%	51 Missing ⚠️
devito/mpi/distributed.py	46.87%	17 Missing ⚠️
benchmarks/user/benchmark.py	46.66%	16 Missing ⚠️
devito/arch/compiler.py	50.00%	15 Missing and 1 partial ⚠️
devito/types/dense.py	75.00%	10 Missing ⚠️
devito/data/data.py	42.85%	8 Missing ⚠️
devito/tools/data_structures.py	50.00%	8 Missing ⚠️
devito/types/grid.py	75.75%	8 Missing ⚠️
devito/types/sparse.py	75.86%	7 Missing ⚠️
devito/ir/stree/tree.py	40.00%	6 Missing ⚠️
... and 55 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2771      +/-   ##
==========================================
- Coverage   83.01%   78.97%   -4.04%     
==========================================
  Files         248      248              
  Lines       51074    50826     -248     
  Branches     4491     4393      -98     
==========================================
- Hits        42401    40142    -2259     
- Misses       7902     9881    +1979     
- Partials      771      803      +32

Flag	Coverage Δ
pytest-gpu-aomp-amdgpuX	`?`
pytest-gpu-nvc-nvidiaX	`?`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

.github/workflows/lint.yml

FabioLuporini · 2025-10-17T12:37:30Z

pyproject.toml

+# But ignore these inconvenient rules
+ignore = ["F403", "E226", "E731", "E275", "F405", "E722", "E741", "W605"]
+
+[tool.isort]


you look back 10 years ago or so there's a commit with message "FREEEEEEEEEEEEEDOOOMM" in which we are essentially removing isort from devito 😂

What was the motivation for removing?

FabioLuporini · 2025-10-17T12:37:54Z

pyproject.toml

+[tool.isort]
+line_length = 90
+known_first_party = ["devito", "examples"]
+multi_line_output = "VERTICAL_GRID_GROUPED"


if this is for

from X import (
A,
B,
C,
...
)

then I vote no

No this is in direct response to your comment on my other PR. This is specifically to avoid using ruff for the import formatting (see this issue)

pyproject.toml

.github/workflows/lint.yml

review-notebook-app · 2025-11-13T20:01:53Z

Check out this pull request on

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

JDBetteridge · 2026-01-05T14:12:27Z

This is almost ready, it doesn't look like it, but I will merge these two auxiliary branches in once I've tidied the commits and the second is green...

#2813

#2814

…anges_part2 misc: ruff manual changes part2

…anges misc: Ruff manual changes

This reverts commit 1318b52.

mloubout · 2026-01-06T16:28:09Z

devito/core/cpu.py

-           'Cpu64AdvOmpOperator', 'Cpu64FsgCOperator', 'Cpu64FsgOmpOperator',
-           'Cpu64CustomOperator', 'Cpu64CustomCXXOperator', 'Cpu64AdvCXXOperator',
-           'Cpu64AdvCXXOmpOperator', 'Cpu64FsgCXXOperator', 'Cpu64FsgCXXOmpOperator']
+__all__ = [


Why are some one per line and some are not (like archinfo.py)

Because I put a # noqa: RUF022 around the line in archinfo.py since the list is carefully arranged and documented.

Note that isort fixes the imports and ruff formats the __all__.

mloubout · 2026-01-06T16:29:08Z

devito/data/allocators.py

 import mmap
 import os
 import sys
+from ctypes.util import find_library


Slightly weird that it import ctypes then others then ctypes.util

It [ruff/isort] did not do that, we imported both in this way and then it just got formatted like so. We should just fix it 😅 (which I will takes as the action to this comment)

Nevermind! There is a good reason for it being the way it was! (util is a module and is not automatically imported when you import ctypes)

EdCaunt · 2026-01-07T09:49:44Z

.github/actionlint.yaml

Nitpick: this file is a .yaml whilst all the others are .yml

EdCaunt · 2026-01-07T09:53:10Z

benchmarks/user/benchmark.py

    """
    Initialize any necessary input and run the operator associated with the solver.
    """
    # Get the operator if exist


Nitpick, but whilst we're fixing all the typos "Get the operator if it exists"

EdCaunt · 2026-01-07T10:25:04Z

devito/core/operator.py


        if oo['mpi'] and oo['mpi'] not in cls.MPI_MODES:
-            raise InvalidOperator("Unsupported MPI mode `%s`" % oo['mpi'])
+            raise InvalidOperator("Unsupported MPI mode `{}`".format(oo['mpi']))


Suggested change

raise InvalidOperator("Unsupported MPI mode `{}`".format(oo['mpi']))

raise InvalidOperator(f"Unsupported MPI mode `{oo['mpi']}`")

EdCaunt · 2026-01-07T11:35:44Z

devito/finite_differences/derivative.py

        psubs = {}
        nx0 = x0.copy()
-        for d, d0 in x0.items():
+        for d, _ in x0.items():


Suggested change

for d, _ in x0.items():

for d in x0:

EdCaunt · 2026-01-07T13:17:51Z

devito/ir/clusters/visitors.py

                func, mode = args
            else:
-                assert False
+                raise AssertionError('Too many args')


Suggested change

raise AssertionError('Too many args')

raise ValueError(f"Either 1 or 2 arguments permitted, {len(args)} provided")

EdCaunt · 2026-01-07T14:07:28Z

devito/ir/iet/nodes.py


    def __repr__(self):
-        return "<SyncSpot (%s)>" % ",".join(str(i) for i in self.sync_ops)
+        return "<SyncSpot ({})>".format(",".join(str(i) for i in self.sync_ops))


Suggested change

return "<SyncSpot ({})>".format(",".join(str(i) for i in self.sync_ops))

return f"<SyncSpot ({','.join(str(i) for i in self.sync_ops)})>"

EdCaunt · 2026-01-07T14:08:37Z

devito/ir/iet/nodes.py

    def __repr__(self):
-        functions = "(%s)" % ",".join(i.name for i in self.functions)
-        return "<%s%s>" % (self.__class__.__name__, functions)
+        functions = "({})".format(",".join(i.name for i in self.functions))


Suggested change

functions = "({})".format(",".join(i.name for i in self.functions))

functions = f"({','.join(i.name for i in self.functions)})"

EdCaunt · 2026-01-07T14:10:02Z

devito/ir/iet/visitors.py

                v = f._C_name
            else:
-                assert False
+                raise AssertionError('rvalue is not a recognised type')


Suggested change

raise AssertionError('rvalue is not a recognised type')

raise TypeError('rvalue is not a recognised type')

EdCaunt · 2026-01-07T14:10:29Z

devito/ir/iet/visitors.py

                    v = f._C_field_dmap
                else:
-                    assert False
+                    raise AssertionError('rvalue is not a recognised type')


Suggested change

raise AssertionError('rvalue is not a recognised type')

raise TypeError('rvalue is not a recognised type')

EdCaunt · 2026-01-07T14:13:49Z

devito/ir/stree/algorithms.py

    try:
        assert not queue
-    except AssertionError:
+    except AssertionError as e:
        if options['mpi']:
-            raise RuntimeError("Unsupported MPI for the given equations")
+            raise RuntimeError("Unsupported MPI for the given equations") from e


Suggested change

try:

assert not queue

except AssertionError:

except AssertionError as e:

if options['mpi']:

raise RuntimeError("Unsupported MPI for the given equations")

raise RuntimeError("Unsupported MPI for the given equations") from e

if not queue and options['mpi']:

raise RuntimeError("Unsupported MPI for the given equations")

EdCaunt

Looks mainly good to me - feel free to ignore any and all of my comments since they don't affect functionality and it's still an improvement over the existing code.

We should just get this merged to avoid hellish rebasing and then fix the quibbles later

github-advanced-security bot found potential problems Oct 16, 2025

View reviewed changes

.github/workflows/lint.yml Fixed Show fixed Hide fixed

.github/workflows/lint.yml Fixed Show fixed Hide fixed

.github/workflows/lint.yml Fixed Show fixed Hide fixed

JDBetteridge requested a review from EdCaunt October 16, 2025 23:17

JDBetteridge added CI continuous integration misc python Pull requests that update python code github_actions Pull requests that update GitHub Actions code labels Oct 16, 2025

github-advanced-security bot found potential problems Oct 17, 2025

View reviewed changes

.github/workflows/lint.yml Fixed Show fixed Hide fixed

FabioLuporini reviewed Oct 17, 2025

View reviewed changes

mloubout approved these changes Oct 17, 2025

View reviewed changes

EdCaunt reviewed Oct 17, 2025

View reviewed changes

pyproject.toml Show resolved Hide resolved

github-advanced-security bot found potential problems Oct 17, 2025

View reviewed changes

.github/workflows/lint.yml Fixed Show fixed Hide fixed

github-advanced-security bot found potential problems Oct 17, 2025

View reviewed changes

.github/workflows/lint.yml Fixed Show fixed Hide fixed

JDBetteridge self-assigned this Nov 7, 2025

JDBetteridge force-pushed the JDBetteridge/use_ruff branch 4 times, most recently from a4609a1 to 43a044d Compare November 13, 2025 18:29

JDBetteridge force-pushed the JDBetteridge/use_ruff branch 4 times, most recently from 70151d8 to b19cb9d Compare December 24, 2025 16:53

JDBetteridge added 5 commits January 5, 2026 15:45

ci: Add Ruff linting

de13162

ci: Add actionlint and hadolint for good measure

35861db

ci: Give codelint a name

991fa90

misc: Replace ruff I rules with more specific isort rules

1c37d58

misc: Add support for command line spellchequing with typo

57f0d3f

JDBetteridge added 9 commits January 5, 2026 15:49

misc: Fix typos in Devito

e2ffe37

misc: Fix typos in examples

b450d67

misc: Fix typos in tests

680f1b5

misc: Kick CI

74243a7

misc: Manual linting for Devito

67d7c08

misc: Automated linting fixes in examples, tests, scripts

26c3580

misc: Manually lint examples, tests, scripts, etc

d3b879c

misc: Update precommit to include flake8

380f399

misc: Various fixes

540a43f

JDBetteridge force-pushed the JDBetteridge/use_ruff branch from 9b5069c to 1ec3c7f Compare January 6, 2026 11:25

JDBetteridge and others added 6 commits January 6, 2026 11:27

Merge pull request #2814 from devitocodes/JDBetteridge/ruff_manual_ch…

c91949e

…anges_part2 misc: ruff manual changes part2

Merge pull request #2813 from devitocodes/JDBetteridge/ruff_manual_ch…

5c48147

…anges misc: Ruff manual changes

ci: Change linting order to run ruff before flake8

977c550

misc: Update contributing guidelines to include pre-commit

c3bc3f8

Revert "misc: Deliberately add a file with a typo"

9893ad8

This reverts commit 1318b52.

misc: Fix helper script

9595621

JDBetteridge requested review from EdCaunt, FabioLuporini, ZoeLeibowitz, ggorman and mloubout January 6, 2026 16:20

mloubout reviewed Jan 6, 2026

View reviewed changes

JDBetteridge force-pushed the JDBetteridge/use_ruff branch from 795c02b to ac035e1 Compare January 6, 2026 17:15

misc: Remove double import of ctypes in allocators

83d7f50

JDBetteridge force-pushed the JDBetteridge/use_ruff branch from ac035e1 to b19bb09 Compare January 6, 2026 17:22

misc: Isolated fixes that only break things in PRO

8d1fa60

JDBetteridge force-pushed the JDBetteridge/use_ruff branch from b19bb09 to 8d1fa60 Compare January 6, 2026 17:33

EdCaunt reviewed Jan 7, 2026

View reviewed changes

EdCaunt approved these changes Jan 7, 2026

View reviewed changes

	raise InvalidOperator("Unsupported MPI mode `{}`".format(oo['mpi']))
	raise InvalidOperator(f"Unsupported MPI mode `{oo['mpi']}`")

	raise AssertionError('Too many args')
	raise ValueError(f"Either 1 or 2 arguments permitted, {len(args)} provided")

	return "<SyncSpot ({})>".format(",".join(str(i) for i in self.sync_ops))
	return f"<SyncSpot ({','.join(str(i) for i in self.sync_ops)})>"

	functions = "({})".format(",".join(i.name for i in self.functions))
	functions = f"({','.join(i.name for i in self.functions)})"

	raise AssertionError('rvalue is not a recognised type')
	raise TypeError('rvalue is not a recognised type')

misc: Ruff #2771

Are you sure you want to change the base?

misc: Ruff #2771

Uh oh!

Conversation

JDBetteridge commented Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coding guidelines

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

review-notebook-app bot commented Nov 13, 2025

Uh oh!

JDBetteridge commented Jan 5, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

EdCaunt Jan 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

EdCaunt left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

JDBetteridge commented Oct 16, 2025 •

edited

Loading

codecov bot commented Oct 16, 2025 •

edited

Loading

EdCaunt Jan 7, 2026 •

edited

Loading