Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

basic flake8 stuff #6629

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

basic flake8 stuff #6629

wants to merge 6 commits into from

Conversation

wxtim
Copy link
Member

@wxtim wxtim commented Feb 24, 2025

Apply basic Flake 8 standards to the tests.

  • I have read CONTRIBUTING.md and added my name as a Code Contributor.
  • Contains logically grouped changes (else tidy your branch by rebase).
  • Does not contain off-topic changes (use other PRs for other changes).
  • Applied any dependency changes to both setup.cfg (and conda-environment.yml if present).
  • Change to the linting of the tests, so no test changes, doc changes, changelog entries are required.
  • Raised against master because it's a biggish change. Happy to rebase.

@wxtim wxtim added the small label Feb 24, 2025
@wxtim wxtim requested a review from oliver-sanders February 24, 2025 16:46
@wxtim wxtim self-assigned this Feb 24, 2025
hjoliver
hjoliver reviewed Feb 24, 2025
View reviewed changes
oliver-sanders
oliver-sanders reviewed Feb 26, 2025
View reviewed changes
tox.ini Outdated
Comment on lines 22 to 26
tests/*: A001, A003, B010, B017, B904, C401, C405, C408, C413, C416, C420, E731, TC001, TC002, TC003, ISC001, N400, SIM117, SIM201, SIM203, SIM300


; for clarity we don't merge 'with Conf():' context trees
tests/unit/parsec/*: SIM117, TC001, TC002, TC003
tests/unit/parsec/*: B017, B904, C408, C413, SIM113, SIM117, TC001, TC002, TC003
Copy link
Member

@oliver-sanders oliver-sanders Feb 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than ignoring specific codes, can we just ignore the flake8 extensions as a whole.

E.g:

tests/*: SIM

Rather than:

tests/*: SIM201, SIM203, SIM300

Suggest only using the default flake8 codes for the tests (i.e. turning off all the extensions).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Except Bugbear 904, apparently.

oliver-sanders
oliver-sanders reviewed Feb 26, 2025
View reviewed changes
oliver-sanders
oliver-sanders reviewed Feb 26, 2025
View reviewed changes
@wxtim
move old fixtures to the module in which they are used to prevent rel…
ebc9b85 
…ative imports. Added explanation of B904 needing an explicit exception to tox.ini
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oliver-sanders oliver-sanders Awaiting requested review from oliver-sanders

@hjoliver hjoliver Awaiting requested review from hjoliver

At least 1 approving review is required to merge this pull request.

Assignees

@wxtim wxtim

Labels
small
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@wxtim @hjoliver @oliver-sanders