-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
adding curses.ascii, curses.panel and curses.textpad modules #1792
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Any thoughts on why the python 2.7 tests failed when I was only working in python 3? |
The "python 2.7" tests are actually tests using pytype, which happens to run in Python 2.7. It seems to be angry because you're referring to stubs for _curses but it can't find any. |
stdlib/3/curses/ascii.pyi
Outdated
@@ -0,0 +1,69 @@ | |||
from typing import Sequence, Union, overload | |||
|
|||
NUL: int = ... |
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.
You don't need the = ...
part.
removed '= ...' from variable definitions
inside of
I've tried multiple ways of emulating |
Ah, I see now that Should this be done? |
…t.txt just like curses/__init__.pyi because they all import _curses. (This may not be the right thing to do)
Yes, that makes sense (if one part of curses is already in the blacklist, it probably won't hurt pytype to add more). |
stdlib/3/curses/ascii.pyi
Outdated
SP: int | ||
DEL: int | ||
|
||
controlnames: Sequence[int] |
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.
It looks like a List[str]
to me.
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.
Okay, have been a bit fuzzy about when to be more specific and when to be more general ... Then again it could have been because I was just a bit 'fuzzy' in general. Now that you mention it, more specific sounds like the right choice here.
stdlib/3/curses/ascii.pyi
Outdated
|
||
controlnames: Sequence[int] | ||
|
||
def isalnum(c: str) -> bool: ... |
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.
I think all of these also accept int arguments (_ctoi returns ints directly).
stdlib/3/curses/ascii.pyi
Outdated
def isxdigit(c: str) -> bool: ... | ||
def isctrl(c: str) -> bool: ... | ||
def ismeta(c: str) -> bool: ... | ||
@overload |
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.
These could also be done with a TypeVar ranging over int and str. Using overloads is fine too though.
stdlib/3/curses/textpad.pyi
Outdated
|
||
class Textbox: | ||
stripspaces: bool | ||
def __init__(self, w: _CursesWindow) -> None: ... |
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.
Missing the undocumented insert_before
argument (quoting the code: def __init__(self, win, insert_mode=False):
).
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.
Yeah, I was just looking at the documentation. Makes sense to look at the source code to.
stdlib/3/curses/textpad.pyi
Outdated
class Textbox: | ||
stripspaces: bool | ||
def __init__(self, w: _CursesWindow) -> None: ... | ||
def edit(self, validator: Callable[[int], int]) -> 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.
The argument is called validate
and is both optional and Optional (i.e., it should be validate: Optional[Callable[[int], int]] = ...
).
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.
Ah, documentation said edit([validator])
both make sense but source code always wins.
changed controlnames variable to be declared as List[str] instead of Sequence[str]
stdlib/3/curses/ascii.pyi
Outdated
@overload | ||
def ctrl(c: str) -> str: ... | ||
@overload | ||
def ctrl(c: int) -> 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.
Shouldn't this one be int -> int
(like other items around)?
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.
Yes, that's correct. thank-you.
fixed flake8 issue related to needing and underscore before a private TypeVar
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 making the changes! Here are a few more comments.
stdlib/3/curses/ascii.pyi
Outdated
def ascii(c: _Ch) -> _Ch: ... | ||
def ctrl(c: _Ch) -> _Ch: ... | ||
def alt(c: _Ch) -> _Ch: ... | ||
def unctrl(c: _Ch) -> 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.
This should just be Union[str, int] instead a typevar, because it doesn't influence the return type.
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.
Okay, Starting to understand the TypeVar now.
stdlib/3/curses/ascii.pyi
Outdated
|
||
controlnames: List[int] | ||
|
||
def isalnum(c: str) -> bool: ... |
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.
all these is*
functions take Union[str, int]
(as mentioned in the previous review)
also change unctrl to also take the same. (instead of the inapropriate TypeVar)
* python/master: (269 commits) allow Optional[float] for asyncio.wait() timeout arg (python#1860) Clean up the pytype blacklist. (python#1851) filter function: make the callable of the first overload non-optional so that mypy is able to select the second overload for the case. (python#1855) Accept Text in distutils *Version classes (python#1854) Add attrs library (python#1838) Add type stub for the lzma module (python#1844) Add ImportError attributes name, path for Python 3.3+ (python#1849) Add 'message' to click.decorators.version_option (python#1845) PEP 553 and PEP 564 (python#1846) Adding py36 additions to datetime module (python#1848) Remove incomplete thrift stubs that cause false positives. (python#1827) adding curses.ascii, curses.panel and curses.textpad modules (python#1792) Fix requests session hooks type (python#1794) Add StringIO.name and BytesIO.name (python#1802) Fix werkzeug environ type (python#1831) Change the return type of unittest.TestCase.fail() to NoReturn (python#1843) _curses.tparm works on bytes, not str (python#1828) Refine types for distutils.version (python#1835) Add stub for stat.filemode (python#1837) modify pytype_test to run from within pytype too, and support python3 (python#1817) ...
adding these three stub files to complete the curses module stubs. Some notes on this in issue #1775.