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

Enable F401 in flake8, configure per-file disables. #2669

Merged
merged 6 commits into from
Jul 2, 2023
Merged

Enable F401 in flake8, configure per-file disables. #2669

merged 6 commits into from
Jul 2, 2023

Conversation

jakkdl
Copy link
Member

@jakkdl jakkdl commented Jun 25, 2023

See discussion in #2666 (comment)

@codecov
Copy link

codecov bot commented Jun 25, 2023 •
edited
Loading

Codecov Report

Merging #2669 (cc8047c) into master (c758fbc) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #2669      +/-   ##
==========================================
- Coverage   98.84%   98.84%   -0.01%     
==========================================
  Files         114      113       -1     
  Lines       16514    16477      -37     
  Branches     3003     3003              
==========================================
- Hits        16323    16286      -37     
  Misses        134      134              
  Partials       57       57
Impacted Files Coverage Δ
trio/_core/_exceptions.py 100.00% <ø> (ø)
trio/_core/_multierror.py 100.00% <ø> (ø)
trio/_core/_tests/test_asyncgen.py 99.06% <ø> (-0.01%) ⬇️
trio/_tests/test_abc.py 100.00% <ø> (ø)
trio/_tests/test_exports.py 96.29% <ø> (-0.02%) ⬇️
trio/_tests/test_socket.py 99.83% <ø> (-0.01%) ⬇️
trio/_tests/test_ssl.py 99.86% <ø> (-0.01%) ⬇️
trio/_tests/tools/test_gen_exports.py 100.00% <ø> (ø)
trio/_wait_for_object.py 100.00% <ø> (ø)
trio/_channel.py 100.00% <100.00%> (ø)
... and 15 more

@jakkdl jakkdl force-pushed the enable_f401 branch 2 times, most recently from 5a36d77 to 4b0ac83 Compare June 25, 2023 08:26
jakkdl
jakkdl commented Jun 25, 2023
View reviewed changes
trio/_core/_tests/test_run.py Outdated Show resolved Hide resolved
@jakkdl jakkdl marked this pull request as ready for review June 25, 2023 08:39
@jakkdl
Copy link
Member Author

jakkdl commented Jun 26, 2023

Any objection to just merging this? It'll quickly lead to a lot of merge conflicts if I start opening type-checking PRs in parallel to it.

A5rocks
A5rocks reviewed Jun 27, 2023
View reviewed changes
Copy link
Contributor

@A5rocks A5rocks left a comment

Choose a reason for hiding this comment

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

Looks mostly good. I don't quite get why anything needs to have this warning ignored for (unless it's doing some runtime stuff? which kinda sucks but ig that's a different issue to fix)

.flake8 Outdated Show resolved Hide resolved
trio/_core/_tests/test_util.py Outdated Show resolved Hide resolved
@jakkdl
Copy link
Member Author

jakkdl commented Jun 28, 2023

Looks mostly good. I don't quite get why anything needs to have this warning ignored for (unless it's doing some runtime stuff? which kinda sucks but ig that's a different issue to fix)

added comments to most of the noqa's.

@jakkdl jakkdl force-pushed the enable_f401 branch 2 times, most recently from dd708ac to 256eb67 Compare June 28, 2023 13:43
A5rocks
A5rocks approved these changes Jun 28, 2023
View reviewed changes
Copy link
Contributor

@A5rocks A5rocks left a comment

Choose a reason for hiding this comment

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

Other than these 2 + fixing the formatting, this looks good!

check.sh Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
@A5rocks
Copy link
Contributor

A5rocks commented Jun 29, 2023

Formatting CI is still failing :(

I would be a +1 on adding a nicer error message (e.g. putting a git diff *-requirements.txt or something) in the failure case for pip-compile.

@jakkdl
Copy link
Member Author

jakkdl commented Jun 30, 2023 •
edited
Loading

I'm inclined to ignore the failing pypy-3.7 export symbol test since support will be dropped soon anyway, it does not seem to be related to this PR anyway - main is failing too https://github.com/python-trio/trio/actions/runs/5405876855/jobs/9822036832

@A5rocks
Copy link
Contributor

A5rocks commented Jun 30, 2023

Could you just toss an xfail at it I guess, it's required for merging :(

@A5rocks A5rocks mentioned this pull request Jun 30, 2023
Closed
@A5rocks
Copy link
Contributor

A5rocks commented Jun 30, 2023

python/typing_extensions#258 heh. I'll make a PR.

A5rocks
A5rocks approved these changes Jul 2, 2023
View reviewed changes
Copy link
Contributor

@A5rocks A5rocks left a comment

Choose a reason for hiding this comment

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

Rereviewing, seems good to me.

@A5rocks A5rocks enabled auto-merge (squash) July 2, 2023 01:57
@A5rocks A5rocks merged commit d01158b into python-trio:master Jul 2, 2023
@jakkdl jakkdl deleted the enable_f401 branch July 3, 2023 10:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@A5rocks A5rocks A5rocks approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jakkdl @A5rocks