Ruff

[RMeli]

Suggested in #4810. Enable ruff with very minimal checks B006 and B008. Currently blocked by #4810.

This can be eventually expanded once the re-formatting with black is over. Related to #2450.

PR Checklist

• Tests?
• Docs?
• CHANGELOG updated?
• Issue raised/referenced?

Developers certificate of origin

• I certify that this contribution is covered by the LGPLv2.1+ license as defined in our LICENSE and adheres to the Developer Certificate of Origin.

---

📚 Documentation preview 📚: https://mdanalysis--4825.org.readthedocs.build/en/4825/

[RMeli]

The current failure with ruff is expected, it will be fixed by #4810.

[RMeli]

@MDAnalysis/coredevs please speak up if you have something against this addition. See #2450 and #4810 (review) for context.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.40%. Comparing base (557f27d) to head (7ca23d0).
Report is 3 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #4825      +/-   ##
===========================================
+ Coverage    86.08%   86.40%   +0.31%     
===========================================
  Files          177      189      +12     
  Lines        21742    22809    +1067     
  Branches      3055     3055              
===========================================
+ Hits         18717    19708     +991     
- Misses        2593     2668      +75     
- Partials       432      433       +1     

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

[IAlibay]

@RMeli could you please add it to the agenda for the next business?

I'm blocking because with these things we really want folks to know what the tool is doing and the expected workflow when something happens. It's best to take it slow here, unless there's a rush.

[orbeckst]

Can you please briefly summarize what the PR is accomplishing and what the implications are?

I have no idea what ruff is.

[RMeli]

Can you please briefly summarize what the PR is accomplishing

PR #4810 removes mutable data structures in function calls. This was already done a long time ago in #590, but since no checks were in place the issue creeped back into the codebase. This PR enable such checks, so that this is caught during PRs.

to know what the tool is doing and the expected workflow when something happens

In this PR, ruff is checking that rules B006 and B008 from flake8-bugbear (B) are respected.

B006 | mutable-argument-default | Do not use mutable data structures for argument defaults
B008 | function-call-in-default-argument | Do not perform function call {name} in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable

The expected workflow if something happens is to ask the developer submitting the PR to fix the issue as part of the review process.

I have no idea what ruff is.

Ruff An extremely fast Python linter and code formatter, written in Rust. Because it's fast and encompass the checks of many other tools, it has seen very rapid adoption across many Python projects.

In this PR I only enable the two rules mentioned above, as suggested by @tylerjereddy. Once the formatting PRs are over, we can eventually think of introducing more checks (see rules).

@RMeli could you please add it to the agenda for the next business?

I can do that, although I think this is somewhat uncontroversial (it's only two checks enabled), and might be better discussed asynchronously instead of taking up precious meeting time.

[RMeli]

FYI, this is what the linters GitHub action would flag with this PR (and what #4810 fixes):

package/MDAnalysis/topology/ITPParser.py:432:66: B006 Do not use mutable data structures for argument defaults
    |
430 |         return atom_order, new_params, molnums, self.moltypes, residx
431 | 
432 |     def add_param(self, line, container, n_funct=2, funct_values=[]):
    |                                                                  ^^ B006
433 |         """Add defined GROMACS directive lines, only if the funct is in ``funct_values``"""
434 |         values = line.split()
    |
    = help: Replace with `None`; initialize within function

[tylerjereddy]

Seems "ok" to me, though it was my suggestion to begin with. Makes sense that folks want to take it slow.

Some community examples of usage:

• SciPy: https://github.com/scipy/scipy/blob/main/tools/lint.toml#L22
• scikit-learn: https://github.com/scikit-learn/scikit-learn/blob/main/pyproject.toml#L151
• pandas: https://github.com/pandas-dev/pandas/blob/main/pyproject.toml#L210
• the proposal to switch to it in NumPy: Linting Package Replacement: Pycodestyle to Ruff numpy/numpy#24994

I have no idea what ruff is.

That's fair, their tooling (https://astral.sh/) is nice. I did see some criticism about how they got their funding or something like that, but so far the experience has been fast/very good tooling (their uv tool is also lightning fast for managing Python envs--I use it to install free-threaded CPython 3.13t, which is otherwise a bit annoying to do on some platforms).

[IAlibay]

Thanks for the explanation, if it's reasonable / ok with folks, I would still like to ask that this go up at the next business meeting though.

[RMeli]

Ok, will do.

[orbeckst]

Thank you for the explanation @RMeli . Sounds sensible to me.

I don't think that the PR needs to be closed. As long as @IAlibay 's review blocks it, it won't get merged. I would reopen it but because I don't really know your thinking behind closing it, I'll refrain.

[RMeli]

I closed simply because we don't yet know when the next developer meeting is (unless I missed something) and I'm not sure I can participate, so things might slip of months. For this reason I prefer not to pollute the PR board; it's easy to re-open if we decide we want to, and I can re-open while incorporating eventual feedback.

If you prefer to have it open, I don't mind either way since it does not affect the files touched by black anyways.

[tylerjereddy]

I'll keep an eye out for the next business meeting.

[orbeckst]

I added a discussion item to the agenda for the next meeting.