[MAINT] add workflow to check for f strings

[Remi-Gau]

Closes #2528
Relates to https://github.com/orgs/nilearn/projects/2/views/1?pane=issue&itemId=30677086

Changes proposed in this pull request:

• add workflow to run flynt on main and PRs
• add flynt config to pyproject.toml
• run flynt

[github-actions]

👋 @Remi-Gau Thanks for creating a PR!

Until this PR is ready for review, you can include the [WIP] tag in its title, or leave it as a github draft.

Please make sure it is compliant with our contributing guidelines. In particular, be sure it checks the boxes listed below.

• PR has an interpretable title.
• PR links to Github issue with mention Closes #XXXX (see our documentation on PR structure)
• Code is PEP8-compliant (see our documentation on coding style)
• Changelog or what's new entry in doc/changes/latest.rst (see our documentation on PR structure)

For new features:

• There is at least one unit test per new function / class (see our documentation on testing)
• The new feature is demoed in at least one relevant example.

For bug fixes:

• There is at least one test that would fail under the original bug conditions.

We will review it as quick as possible, feel free to ping us with questions if needed.

[codecov]

Codecov Report

Merging #3759 (15a7633) into main (bb59d69) will not change coverage.
The diff coverage is 0.00%.

@@           Coverage Diff           @@
##             main    #3759   +/-   ##
=======================================
  Coverage   91.65%   91.65%           
=======================================
  Files         139      139           
  Lines       16554    16554           
  Branches     3230     3230           
=======================================
  Hits        15172    15172           
  Misses       1379     1379           
  Partials        3        3           

Impacted Files	Coverage Δ
nilearn/_utils/data_gen.py	98.43% <ø> (ø)
nilearn/_utils/docs.py	93.65% <0.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[ymzayek]

So the github workflow will fail if any code is added with % or .format() formatting ?

[Remi-Gau]

So the github workflow will fail if any code is added with % or .format() formatting ?

only for the simplest usage of % and format

in terms of complexity flynt acts like a high-pass filter for string formatting

it has an "aggressive" option to force reformating but the doc itself that those reformating may result in behavior change

does that make sense?

[Remi-Gau]

oh and it is already one the pre-commit hook that is run, this would just provide an official check in CI

[tsalo]

@Remi-Gau have you looked at flake8-use-fstring? I don't know if it's better or worse than flynt, but you'd be able to include it in the flake8 workflow that already exists.

[Remi-Gau]

Was not aware of this one. Will investigate.

[Remi-Gau]

@Remi-Gau have you looked at flake8-use-fstring? I don't know if it's better or worse than flynt, but you'd be able to include it in the flake8 workflow that already exists.

OK as far I can tell the 2 are not completely mutually exclusive. Flynt will actually try to convert % and Format to f strings but can only do the most simple ones because it tries to be conservative.

The flake8 extension will flag % and format and potentially report them as error.

So the 2 could coexist but one will throw more errors than the other when used for Linting purposes.

I would suggest to start with Flynt and and then tackle the remaining non f strings. Then we can switch to flake8 extensions to ensure we stick to f strings from now on.

[tsalo]

👍 sounds good to me. Thanks for taking a look at the flake8 extension.

[bthirion]

The PR LGTM, but I think lots of red CI.

[Remi-Gau]

The PR LGTM, but I think lots of red CI.

yeah I will rebase to clean some of that

[bthirion]

Now it mixed several things but LGTM overall.

[Remi-Gau]

yeah some files should not be in that diff: will fix