Simplify ruff configuration

[vincentmacri]

Also fix a few rules that were only violated a small number of times.

This PR:

• Removes list of rules we want to follow from src/tox.ini to remove duplication
• Defines all rules we wish to follow in pyproject.toml
  • For rulesets that are not stable enough yet (E, PL) we list the rules explicitly instead of enabling all E or all PL rules.
  • We still ignore specific E and PL rules that we don't want to follow because the hope is that once the E and PL rules are fully stable in ruff we will just enable them all and selectively disable the ones we don't want.
• We add a new file .github/workflows/ruff.toml that starts with the pyproject.toml configuration (using extend, so no duplication), and disables all rules that currently fail.
• We also ignore a few troublesome directories in the ruff config, and run ruff on the entire repo. This means tools and scripts are linted now as well.
• I also fixed a handful of rules that were only violated in a couple files, I didn't want to add a rule to the CI ignore list if fixing the rule was a one line change.

The benefits of this are:

• There is now only one file (pyproject.toml) that needs to be modified to add new rules
• We can now locally run the same ruleset as CI with ruff --config .github/workflows/ruff.toml

📝 Checklist

• The title is concise and informative.
• The description explains in detail what this PR is about.
• I have linked a relevant issue or discussion.
• I have created tests covering the changes.
• I have updated the documentation and checked the documentation preview.

⌛ Dependencies

[tobiasdiez]

Thanks a lot, this is a nice improvement. Especially the handling of the rules activated in CI is way cleaner now.

I have a couple of remarks, but nothing serious.

[tobiasdiez]

From my side, we could also delete the ruff stuff in src/tox now completely.

[vincentmacri]

Done, and updated the developer documentation accordingly.

[tobiasdiez]

I think there are actually quite a few imports in methods that could be moved to the top level without a problem. You want method-level imports only in one instance, namely to break circular imports, right? In that case it would be very helpful if the important is annotated to ease migrating to the new lazy imports (which doesn't help with circular imports) and so it's good if they have a comment like
# noqa: PLC0415 - breaks circular import x > y > z
Then during the migration to the built-in lazy imports, you know you don't need to touch those imports.

If it's just a heavy import that is rarely used in the module, it's okay to convert it to a lazy_import (and then later to the built-in lazy import).

[vincentmacri]

In that case it would be very helpful if the important is annotated to ease migrating to the new lazy imports (which doesn't help with circular imports)

Lazy imports can help with circular imports depending on the situation. Sometimes a lazy import can fix a circular import, sometimes it can't.

As for what to do until we can use built-in lazy imports, there are so many linter failures related to imports that I think trying to address is futile unless we can reliably do it automatically, I don't think we can until built-in lazy imports are available.

If it's just a heavy import that is rarely used in the module, it's okay to convert it to a lazy_import (and then later to the built-in lazy import).

Yeah but I don't think it's worth the trouble of fixing thousands of lines for this, just to do it again once built-in lazy imports are available.

[vincentmacri]

For context: PLC0415 currently has 10606 failures and has no autofix option available.

[tobiasdiez]

A couple of weeks ago, I looked into how the migration to lazy import will most likely look like for us (and asked some clarifying questions to the PEP authors). Python will provide some way to call the underlying logic of the lazy import mechanism (similar to the dynamic import method). We can use this in our lazy_import method using something like

def lazy_import(module):
   if python >= 3.15: return python_builtin_lazy_import(module)
   else: our custom implementation

Then, once we have 3.15 as the minimal Python version (which will be in 4 to 5 years), we can replace lazy_import(...) by lazy import ... globally.
So it's a pretty long time frame until we can really profit from the new lazy imports syntax.

As for the local imports, the overall migration to lazy imports would look like:

• Local import > global lazy_import
• lazy_import > lazy import

Local imports that cannot be migrated to lazy imports, would need to be annotated in either case.

As for the reason why I like to keep PCL0415 activated is that IDEs highlight the import as a linter warning. So if you touch the surrounding code, you get a natural push to move the local import to a global (lazy) import. Then you usually run the doctests anyway, and will see if the import can be global or it really needs to be local (to break a circular import). I like to keep that nudge, as we would like to remove all possible local imports in the long term anyway.

[tobiasdiez]

I don't quite understand the lazy_import argument. Can you not just have all normal imports at the top, then the lazy imports afterwards?

(We might want to defer this rule until there is a decision on how lazy import should be grouped/sorted - if they get a separate group anyway, that would be a good reason to already follow E402; if they are not grouped separately, then it would only introduce additional noise to first follow E402).

[vincentmacri]

I don't quite understand the lazy_import argument. Can you not just have all normal imports at the top, then the lazy imports afterwards?

Sure, but I don't think it's worth the amount of effort to do that reformatting and then do it again if we decide to replace our custom lazy import system with the PEP 810 one.

[tobiasdiez]

Note that we use a fixed version of ruff in CI, so I would say the danger of random failures due to rules in preview is relatively low.
(But it's fine with me as well to list the rules explicitly as you do here)

[vincentmacri]

I have a different version ruff installed in my system than we fix in the CI, and found things passing locally then failing on CI because new rules were available on CI. I thought this might be annoying for developers to deal with, so this minimizes that risk. But if you think we should enable more rules then I'm okay with that if the CI version is fixed, but wouldn't that make it more work to do PRs that update our dependencies?

[tobiasdiez]

Good point, I forgot about the possible version mismatch when running it locally. Then listing the rules like you did is indeed a good idea!

[github-actions]

Documentation preview for this PR (built with commit fe61df1; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

[vincentmacri]

Updated part of the docs: https://doc-pr-41259--sagemath.netlify.app/html/en/developer/tools#ruff