Hi Andrew can you give me access rights for this PR please ? 😄

🚀 GPU test workflows successfully triggered!

Motivations for the ignores of each rules:

• E402 (module level import not a top of file) and TID253 (banned-module-level-import) are ignored in examples to allow for imports at any steps of the notebook, which makes the example easier to read.
• SLF001 (private member accessed) is ignored in deepinv/transform/base.py because we need access to private pytorch methods there. It is also ignored in tests to allow for easy testing of private methods.
• F811 (redefinition of unused name) is ignored in tests because it doesn't work well with fixtures. The linter doesn't understand that the imported fixture is actually used when using it as argument to a test.
• F401, E402, F403, F405 (all related to star imports) are ignored in deepinv.__init__.py, I'm not really sure if it's possible to avoid star imports there.
• Another notable ignore of F401 (star import) is in deepinv/optim/optimizers.py. The optim builder uses class attribute __name__ to instantiate iterators. Importing the iterators explicitly raises a linting error (unused import) because they are not explicitly used in the code. A workaround could be using a registry containing explicit iterator class names, but that creates a dependency between this file and the addition of new iterators. Overall I decided that this star import was fine.
• Exactly the same star import issue for sampling iterators, which I think is fair to ignore too for now. This probably points to a deeper design issue with the builder classes (both sampling and optim)

Codecov Report

❌ Patch coverage is 85.02304% with 65 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.66%. Comparing base (9018462) to head (26823c5).
⚠️ Report is 3 commits behind head on main.
✅ All tests successful. No failed tests found.

@@            Coverage Diff             @@
##             main     #705      +/-   ##
==========================================
+ Coverage   84.61%   84.66%   +0.05%     
==========================================
  Files         208      208              
  Lines       21281    21345      +64     
  Branches     2893     2889       -4     
==========================================
+ Hits        18006    18072      +66     
+ Misses       2359     2355       -4     
- Partials      916      918       +2     

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

I think this is ready for review, check my comment above for more details / motivations behind some linting exceptions. While a large portion of the changes were performed automatically by ruff, it still took a lot of manual fixes to get all these rules to pass. In the future it'll probably be better to enforce more specific sets of rules within a single PR.

I agree that the PR is very hard to review between automatic and manual fixes. Dividing it into multiple PRs would be better but I'm afraid that I made the CI pass only in the last few commits, so taking only previous commits might break things.

@Tmodrzyk Yeah it would definitely be easier if we could just extract the commits here but I have a couple other tricks up my sleeve I think it's going to be doable. I'll start with extracting stuff related to E402 first we'll see how it goes!

Quick questions:

• I see A005 was unlisted in the merge commit ffcc0f2 I think it is a mistake?
• You mention TID253 in your message but it doesn't look like it was changed in any way, am I missing something?

Hi @jscanvic !

• TID253 was already in the rule set, but it is know ignored in /examples to allow for more flexible placements of the imports.
• A005 being unlisted was indeed a mistake, I've added it back to the rule set

OOps I think I did an error with the last commit ?

But it is already ignored in the main branch, not that it matters too much

Thanks for adding the rule that was dropped!

I'll have a look shortly