Minor refactors in _threads.py

[richardsheridan]

Since I've been staring at the threading code for a long time now, I've found a couple of changes that are probably good for maintainability and good style, but were not a priority for release. I recommend looking at this PR commit by commit rather than as a whole; I'll try to make sure CI gives a green check for each discrete refactor.

[codecov]

Codecov Report

Merging #2871 (9f1c008) into master (d5c0fb8) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2871      +/-   ##
==========================================
- Coverage   99.51%   99.51%   -0.01%     
==========================================
  Files         115      115              
  Lines       17683    17672      -11     
  Branches     3172     3171       -1     
==========================================
- Hits        17598    17587      -11     
  Misses         56       56              
  Partials       29       29              

Files	Coverage Δ
src/trio/_threads.py	100.00% <100.00%> (ø)

[richardsheridan]

Mypy boffins: the diff of 5422f61 doesn't touch the abandon_on_cancel variable but somehow mypy can't tell anymore that the variable cannot be None past line 302. Is this a mypy bug? I'll just ignore it for now.

[A5rocks]

Mypy boffins: the diff of 5422f61 doesn't touch the abandon_on_cancel variable but somehow mypy can't tell anymore that the variable cannot be None past line 302. Is this a mypy bug? I'll just ignore it for now.

mypy doesn't let you redefine the variable type. however, you can stick a assert abandon_on_cancel is not None because mypy narrowing will work.

I'm not sure why it would change if you didn't change the code related though...

[richardsheridan]

I'd rather ignore than assert because

• if a None sneaks through, it likely has the correct semantics, so an assertion would be unnecessary brittleness
• we can save a bytecode on every function call for something we can determine statically

Happy to hear an argument otherwise though!

[A5rocks]

I don't see why mypy started warning you just now, so that's probably a sign of a mypy bug.

As for adding an assert, people can always use -O and get rid of them (... assuming trio doesn't rely on asserts having side effects, which it really shouldn't) so I regard point 2 as irrelevant and point 1 is impossible AFAIK. But idk if it's worth addressing.

[richardsheridan]

mypy doesn't let you redefine the variable type.

Actually this was the right answer all along, I just needed a new variable to carry along the narrowed type!

[CoolCat467]

You are quite right, looking at as one big change is confusing. After going through each commit, this looks great!

[A5rocks]

One small thing I noticed, looks good other than this.

[A5rocks]

Sorry about the confusion, looks good.

[CoolCat467]

With a second approval, I am going to merge this.