flake8 scans failing on CI/CD pipelines

[maciejskorski]

CI/CD jobs in many PRs are failing when scanning the code with flake8.

The issue seems to break on the base code, not on PR-ed commits. Are flake8 rules pinned?

sympy/tensor/array/expressions/from_array_to_matrix.py:874:16: C416 Unnecessary dict comprehension - rewrite using dict().
Error: Process completed with exit code 1.

See
https://github.com/sympy/sympy/actions/runs/4456494418/jobs/7826977213?pr=24253#step:7:1
https://github.com/sympy/sympy/actions/runs/4456527846/jobs/7827030982
https://github.com/sympy/sympy/actions/runs/4456328202/jobs/7826700508

[hanspi42]

This might be related to #24291

[maciejskorski]

This might be related to #24291

While that topic discusses tests, here we are failing during a flake8 scan.
Do you mean something specific?

[hanspi42]

I think I attributed it incorrectly, sorry

[oscarbenjamin]

The C4 error codes were enabled a few weeks ago in gh-24741. It looks like there was a new release (3.11.0) of flake-comprehensions 12 hours ago:
https://pypi.org/project/flake8-comprehensions
The changelog says that C416 is a new rule added in that release (apparently by a SymPy contributor @Skylion007):
https://github.com/adamchainz/flake8-comprehensions/blob/main/CHANGELOG.rst
The new release will automatically be picked up in SymPy's CI because CI doesn't pin versions of these things.

To reproduce locally you should update flake8-comprehensions:

pip install -U flake8 flake8-comprehensions

We could pin the version of flake8-comprehensions in CI but it is good to get updates like this automatically. We could also choose to ignore C416 by setting that in pyproject.toml.

Really though the problems it is complaining about seem like legitimate problems to me e.g.:

sympy/sympy/tensor/array/expressions/from_array_to_matrix.py

Line 874 in 326277c

free_map = {k: v for k, v in zip(editor.args_with_ind, free_indices[:-1])}

That should just be:

free_map = dict(zip(editor.args_with_ind, free_indices[:-1]))

Using dict directly rather than a comprehension is more efficient especially if the arguments to zip are all builtins like a list of strings etc.

I think the best fix is just to open a PR to change the code that flake8-comprehensions is complaining about. Once that is merged other PRs will pass CI.

[Skylion007]

Yeah, I submitted an issue to ruff to add an autofixer, but no one has added it yet: astral-sh/ruff#3598

[maciejskorski]

I think the best fix is just to open a PR to change the code that flake8-comprehensions is complaining about. Once that is merged other PRs will pass CI.

👍 Only about a dozen of lines sharing the same pattern, hence fixable manually.

I have fixed it this way in the PR attached.

[maciejskorski]

Yeah, I submitted an issue to ruff to add an autofixer, but no one has added it yet: charliermarsh/ruff#3598

My two cents are that autofixing may not be reliable. What to do with more sophisticated patterns such as dictionary reversion {v:k for k,v in ...} or dictionary processing {k:f(v) for k,v in }.

[Skylion007]

Yeah, I submitted an issue to ruff to add an autofixer, but no one has added it yet: charliermarsh/ruff#3598

My two cents are that autofixing may not be reliable. What to do with more sophisticated patterns such as dictionary reversion {v:k for k,v in ...} or dictionary processing {k:f(v) for k,v in }.

Those two dict comprehensions are fine and shouldn't flag the C416 rule I wrote anyway so it wouldn't trigger the autofixers anyhow.

[maciejskorski]

Those two dict comprehensions are fine and shouldn't flag the C416 rule I wrote anyway so it wouldn't trigger the autofixers anyhow.

This is also demonstrated by my PR #24937 where all the tests pass after fixing a dozen of C416 patterns.

[maciejskorski]

Those two dict comprehensions are fine and shouldn't flag the C416 rule I wrote anyway so it wouldn't trigger the autofixers anyhow.

On this note: where is a directive enforcing autofix?

[Skylion007]

The autofixer is now available in the master of ruff and I've already tested it and used it to update all my codebases for anyone else having issues.