Skip to content

Modernize codebase to python 3.7+ standard#2480

Merged
harahu merged 3 commits intopython-trio:masterfrom
harahu:pyupgrade
Nov 30, 2022
Merged

Modernize codebase to python 3.7+ standard#2480
harahu merged 3 commits intopython-trio:masterfrom
harahu:pyupgrade

Conversation

@harahu
Copy link
Copy Markdown
Contributor

@harahu harahu commented Nov 16, 2022

This PR is the result of a one-off run of pyupgrade on the trio code base.

For future reference, this was done by running the following command from the project root:

find . -name '*.py' -exec pyupgrade --py3-only --py37-plus {} +

I also ran black in preview mode (black --preview), which mainly results in string cleanup.

@codecov
Copy link
Copy Markdown

codecov Bot commented Nov 16, 2022

Codecov Report

Merging #2480 (df093eb) into master (fc0932a) will increase coverage by 0.86%.
The diff coverage is 98.11%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2480      +/-   ##
==========================================
+ Coverage   97.99%   98.85%   +0.86%     
==========================================
  Files         118      118              
  Lines       16222    16222              
  Branches     3143     3143              
==========================================
+ Hits        15896    16037     +141     
+ Misses        264      131     -133     
+ Partials       62       54       -8     
Impacted Files Coverage Δ
trio/_abc.py 100.00% <ø> (ø)
trio/_core/_asyncgens.py 100.00% <ø> (ø)
trio/_core/_local.py 98.11% <0.00%> (ø)
trio/_core/tests/test_asyncgen.py 99.06% <ø> (ø)
trio/_core/tests/test_multierror.py 100.00% <ø> (ø)
trio/_core/tests/test_run.py 100.00% <ø> (ø)
trio/_threads.py 100.00% <ø> (ø)
trio/_util.py 100.00% <ø> (ø)
trio/tests/test_channel.py 100.00% <ø> (ø)
trio/tests/test_dtls.py 99.79% <ø> (ø)
... and 37 more

@harahu harahu changed the title Run pyupgrade Modernize codebase to python 3.7+ standard Nov 17, 2022
@Fuyukai
Copy link
Copy Markdown
Member

Fuyukai commented Nov 17, 2022

I'm in favour of merging this because I like blank lines and double quotes, but I'm not sure if the higher-ups will like it.

@harahu
Copy link
Copy Markdown
Contributor Author

harahu commented Nov 17, 2022

@Fuyukai My thinking is that you are already using black. So might as well apply it properly across the code base.

@harahu
Copy link
Copy Markdown
Contributor Author

harahu commented Nov 22, 2022

I hope this can be merged. I'm realizing that it might seem daunting to review this, given the diff size, but the tools I've used are conservative w.r.t modifying behavior, so the changes should be relatively safe.

@pquentin
Copy link
Copy Markdown
Member

@Fuyukai as a member, you get to approve and merge this if you like it!

@harahu
Copy link
Copy Markdown
Contributor Author

harahu commented Nov 23, 2022

I hope it can be merged. I want to make further changes on top of this.

Copy link
Copy Markdown
Member

@pquentin pquentin left a comment

Choose a reason for hiding this comment

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

Thanks! Can you please exclude notes-to-self?

@harahu
Copy link
Copy Markdown
Contributor Author

harahu commented Nov 24, 2022

Thanks! Can you please exclude notes-to-self?

@pquentin I've done so now. I get one failing test suite, but I think it might be a case of test flakiness? What is your take?

@pquentin
Copy link
Copy Markdown
Member

Not sure what's going on sorry. I've relaunched the Ubuntu 3.7 tests a few times and it appears to fail consistently which is concerning.

@harahu
Copy link
Copy Markdown
Contributor Author

harahu commented Nov 24, 2022

Not sure what's going on sorry. I've relaunched the Ubuntu 3.7 tests a few times and it appears to fail consistently which is concerning.

I'll see if I can figure out what is going on. Perhaps later today.

@pquentin
Copy link
Copy Markdown
Member

I can confirm that the Ubuntu 3.7 failures are not specific to this pull request.

@harahu
Copy link
Copy Markdown
Contributor Author

harahu commented Nov 28, 2022

I can confirm that the Ubuntu 3.7 failures are not specific to this pull request.

Thanks @pquentin! Was scratching my head over this one.

@pquentin
Copy link
Copy Markdown
Member

pquentin commented Nov 29, 2022

Would you mind opening a pull request that skips this test on Ubuntu and Python 3.7 specifically? I can only imagine that a fix in more recent versions was not backported there. I don't want to invest time in debugging this, especially when Python 3.7 won't be supported anymore in 7 months, and this is blocking us.

@harahu harahu requested a review from pquentin November 30, 2022 13:00
@harahu
Copy link
Copy Markdown
Contributor Author

harahu commented Nov 30, 2022

Looks like this one is finally good to go! Thank you so much for all the help @pquentin!

Copy link
Copy Markdown
Member

@pquentin pquentin left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for your patience.

@pquentin
Copy link
Copy Markdown
Member

(I'll let you click on the green merge button yourself!)

@harahu harahu merged commit 55117ad into python-trio:master Nov 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants