Skip to content

Add types to _signals, test_signals#2794

Merged
jakkdl merged 5 commits intopython-trio:masterfrom
TeamSpen210:type-signals
Sep 13, 2023
Merged

Add types to _signals, test_signals#2794
jakkdl merged 5 commits intopython-trio:masterfrom
TeamSpen210:type-signals

Conversation

@TeamSpen210
Copy link
Contributor

This is fairly straightforward, but I did extract a test helper method into a function. This allows open_signal_receiver() to be typed as producing an AsyncIterator, keeping SignalReceiver internal.

@TeamSpen210 TeamSpen210 added the typing Adding static types to trio's interface label Sep 7, 2023
@codecov
Copy link

codecov bot commented Sep 7, 2023

Codecov Report

Merging #2794 (7f27398) into master (be39867) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2794   +/-   ##
=======================================
  Coverage   98.95%   98.95%           
=======================================
  Files         115      115           
  Lines       17003    17011    +8     
  Branches     3045     3045           
=======================================
+ Hits        16825    16833    +8     
  Misses        123      123           
  Partials       55       55           
Files Changed Coverage Δ
trio/_signals.py 100.00% <100.00%> (ø)
trio/_tests/test_signals.py 100.00% <100.00%> (ø)

Copy link
Member

@CoolCat467 CoolCat467 left a comment

Choose a reason for hiding this comment

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

Looks good to me. If anything, the change removing _pending_signal_count could potentially cause issues if people are using this function even though it basically says not to because it was only there for the tests, but I think this is an acceptable change because:

  1. People shouldn't be using said function anyways
  2. Making it a separate function that isn't publicly exposed is a better solution for the tests than having it be a class function that technically could exposed

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 like a pretty simple file to type. Essentially no comments!

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 good to me.

@jakkdl jakkdl merged commit c16003f into python-trio:master Sep 13, 2023
@TeamSpen210 TeamSpen210 deleted the type-signals branch September 18, 2023 03:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

typing Adding static types to trio's interface

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants