Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork252
added type references to pager.py, mypy produced no errors#936
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.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
thomasballinger left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Thanks for working on this!
It looks like some builds are failing becauselist can't be subscripted, I added a note about why.
When I run mypy, I get a few errors
$ python -m mypybpython/pager.py:57: error: Item "None" of "Optional[IO[bytes]]" has no attribute "write"bpython/pager.py:58: error: Item "None" of "Optional[IO[bytes]]" has no attribute "close"bpython/_internal.py:7: error: Incompatible types in assignment (expression has type "Callable[[str, bool], None]", variable has type "Callable[[str], None]")Found 3 errors in 2 files (checked 64 source files)If you figure out how our environments are different let me know and maybe we can add it to some developer docs.
The bpython/_internal.py file is getting more strictly checked now because it usespage defined in pager.py. Apparently the real pydoc.pager doesn't take that second argument. I'd be fine with atype: ignore for that.
pydoc.pager = page # type: ignorebpython/pager.py Outdated
| defget_pager_command(default="less -rf"): | ||
| defget_pager_command(default:str="less -rf")->list[str]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Unfortunately usinglist anddict in type annotations instead oftyping.List andtyping.Dict wasadded in 3.9 and bpython still supports 3.8 and 3.7, so we can't use these yet; you'll need to import List from typing and use that.
| else: | ||
| curses.endwin() | ||
| try: | ||
| popen=subprocess.Popen(command,stdin=subprocess.PIPE) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
For me this needs a
| popen=subprocess.Popen(command,stdin=subprocess.PIPE) | |
| assertpopen.stdinisnotNone | |
| popen=subprocess.Popen(command,stdin=subprocess.PIPE) |
to silence mypy errors about stdin maybe being None.
thomasballinger commentedNov 2, 2021 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
If you aren't able to get mypy to produce these errors locally, you can see them in the CI runs on this page. |
Frostmaine commentedNov 9, 2021
Linter workflow did fail, however it seemed to only be because I modified _internal.py by adding the suggested # type: ignore to pydoc.pager = page. |
codecov-commenter commentedNov 9, 2021 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Codecov Report
@@ Coverage Diff @@## main #936 +/- ##==========================================+ Coverage 68.11% 68.65% +0.53%========================================== Files 61 62 +1 Lines 9244 9434 +190 ==========================================+ Hits 6297 6477 +180- Misses 2947 2957 +10
Continue to review full report at Codecov.
|
sebastinas commentedNov 9, 2021
Indeed, the |
thomasballinger commentedNov 9, 2021
Thanks for adding these types and fixing these CI issues, this looks good to me. The pypy3 CI failure is failing on main right now too. |
Frostmaine commentedNov 10, 2021 via email
Glad to complete my first contribution to a project that I use daily. Ihope to take the things I learned about the development process andcontinue to contribute in my own small way to this project. Any feedbackon how I have conducted my interactions with you are welcome. Thank you somuch for your time. …On Tue, Nov 9, 2021 at 11:17 AM Thomas Ballinger ***@***.***> wrote: Thanks for adding these types and fixing these CI issues, this looks good to me. The pypy3 CI failure is failing on main right now too. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#936 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AMQSM4JJNZPRYEKCE3TUYZ3ULFCSRANCNFSM5HEVBCTA> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. |
First time adding to a project on github, started off small. Would really appreciate feedback.