Skip to content

type _io_epoll and _io_kqueue#2743

Merged
jakkdl merged 11 commits intopython-trio:masterfrom
jakkdl:typing_improvements_io_epoll
Aug 10, 2023
Merged

type _io_epoll and _io_kqueue#2743
jakkdl merged 11 commits intopython-trio:masterfrom
jakkdl:typing_improvements_io_epoll

Conversation

@jakkdl
Copy link
Member

@jakkdl jakkdl commented Aug 6, 2023

Also generates a type guard in _generated_io_* so they don't raise errors if checked in other ways than mypy -m trio

@codecov
Copy link

codecov bot commented Aug 6, 2023

Codecov Report

Merging #2743 (70fcae9) into master (722f1b5) will increase coverage by 0.00%.
The diff coverage is 97.91%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2743   +/-   ##
=======================================
  Coverage   98.90%   98.91%           
=======================================
  Files         113      113           
  Lines       16686    16713   +27     
  Branches     3025     3030    +5     
=======================================
+ Hits        16504    16531   +27     
  Misses        125      125           
  Partials       57       57           
Files Changed Coverage Δ
trio/_core/_io_kqueue.py 85.60% <92.59%> (+0.11%) ⬆️
trio/_core/_io_epoll.py 100.00% <100.00%> (ø)
trio/_core/_io_windows.py 98.76% <100.00%> (ø)
trio/_core/_run.py 99.24% <100.00%> (ø)
trio/_tests/tools/test_gen_exports.py 100.00% <100.00%> (ø)
trio/_tools/gen_exports.py 100.00% <100.00%> (ø)

@jakkdl
Copy link
Member Author

jakkdl commented Aug 6, 2023

I don't think writing a test for the decorator logic in gen_exports matters, so ignoring those. The lacking patch coverage in kqueue for monitor_kevent is from main

Copy link
Contributor

@TeamSpen210 TeamSpen210 left a comment

Choose a reason for hiding this comment

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

A few minor things.

@@ -122,6 +159,9 @@ def gen_public_wrappers_source(source_path: Path, lookup_path: str) -> str:
# Create the function definition including the body
func = astor.to_source(method, indent_with=" " * 4)
Copy link
Member

Choose a reason for hiding this comment

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

If it's possible to fix the type annotation formatting that would be great but if not I understand

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think it's worth the effort, feel free to attack in a separate PR if you want to.
A possibly better fix is enabling Black on the file and changing how it's checked.

Copy link
Contributor

Choose a reason for hiding this comment

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

Could make gen_exports call black after generating even. But definitely a separate PR.

@A5rocks A5rocks added the typing Adding static types to trio's interface label Aug 8, 2023
@jakkdl
Copy link
Member Author

jakkdl commented Aug 8, 2023

Awesome stuff @TeamSpen210 - I would probably not have bothered myself but this is much much nicer and means we can remove it as an exception from F401 in pyproject.toml

@jakkdl jakkdl mentioned this pull request Aug 8, 2023
@jakkdl jakkdl merged commit 0cb2886 into python-trio:master Aug 10, 2023
@jakkdl
Copy link
Member Author

jakkdl commented Aug 10, 2023

good stuff, thanks @TeamSpen210!

@jakkdl jakkdl deleted the typing_improvements_io_epoll branch August 10, 2023 11:10
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