-
-
Notifications
You must be signed in to change notification settings - Fork 32.5k
bpo-19903: IDLE: Calltips changed to use inspect.signature #2822
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
Changes from 7 commits
3969228
6c8e67b
b4bb5dc
7ba5202
a9de237
39907c7
c495b61
68fccf2
55032cf
134cca4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -123,6 +123,8 @@ def get_entity(expression): | |
_INDENT = ' '*4 # for wrapped signatures | ||
_first_param = re.compile(r'(?<=\()\w*\,?\s*') | ||
_default_callable_argspec = "See source or doc" | ||
_invalid_method = "invalid method signature" | ||
_argument_positional = "('/' marks preceding arguments as positional-only)" | ||
|
||
|
||
def get_argspec(ob): | ||
|
@@ -134,25 +136,30 @@ def get_argspec(ob): | |
empty line or _MAX_LINES. For builtins, this typically includes | ||
the arguments in addition to the return value. | ||
''' | ||
argspec = "" | ||
argspec = default = "" | ||
try: | ||
ob_call = ob.__call__ | ||
except BaseException: | ||
return argspec | ||
if isinstance(ob, type): | ||
fob = ob.__init__ | ||
elif isinstance(ob_call, types.MethodType): | ||
fob = ob_call | ||
else: | ||
fob = ob | ||
if isinstance(fob, (types.FunctionType, types.MethodType)): | ||
argspec = inspect.formatargspec(*inspect.getfullargspec(fob)) | ||
if (isinstance(ob, (type, types.MethodType)) or | ||
isinstance(ob_call, types.MethodType)): | ||
argspec = _first_param.sub("", argspec) | ||
return default | ||
|
||
fob = ob_call if isinstance(ob_call, types.MethodType) else ob | ||
|
||
try: | ||
argspec = str(inspect.signature(fob)) | ||
except ValueError as err: | ||
msg = str(err) | ||
if msg.startswith(_invalid_method): | ||
return _invalid_method | ||
|
||
if '/' in argspec: | ||
"""Using AC's positional argument should add the explain""" | ||
argspec = '\n'.join([argspec, _argument_positional]) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. argspec += '\n' + _argument_positional There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll fix There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Even better. |
||
if isinstance(fob, type) and argspec == '()': | ||
"""fob with no argument, use default callable argspec""" | ||
argspec = _default_callable_argspec | ||
|
||
lines = (textwrap.wrap(argspec, _MAX_COLS, subsequent_indent=_INDENT) | ||
if len(argspec) > _MAX_COLS else [argspec] if argspec else []) | ||
if len(argspec) > _MAX_COLS else [argspec] if argspec else []) | ||
|
||
if isinstance(ob_call, types.MethodType): | ||
doc = ob_call.__doc__ | ||
|
@@ -171,6 +178,7 @@ def get_argspec(ob): | |
argspec = _default_callable_argspec | ||
return argspec | ||
|
||
|
||
if __name__ == '__main__': | ||
from unittest import main | ||
main('idlelib.idle_test.test_calltips', verbosity=2) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -46,25 +46,36 @@ def test_builtins(self): | |
|
||
# Python class that inherits builtin methods | ||
class List(list): "List() doc" | ||
|
||
# Simulate builtin with no docstring for default tip test | ||
class SB: __call__ = None | ||
|
||
def gtest(obj, out): | ||
self.assertEqual(signature(obj), out) | ||
|
||
if List.__doc__ is not None: | ||
gtest(List, List.__doc__) | ||
gtest(List, '(iterable=(), /)\n' + ct._argument_positional + '\n' + | ||
List.__doc__) | ||
gtest(list.__new__, | ||
'Create and return a new object. See help(type) for accurate signature.') | ||
'(*args, **kwargs)\nCreate and return a new object. See help(type) for accurate signature.') | ||
gtest(list.__init__, | ||
'(self, /, *args, **kwargs)\n' + ct._argument_positional + '\n' + | ||
'Initialize self. See help(type(self)) for accurate signature.') | ||
append_doc = "Append object to the end of the list." | ||
gtest(list.append, append_doc) | ||
gtest([].append, append_doc) | ||
gtest(List.append, append_doc) | ||
append_doc = ct._argument_positional + '\n' + "Append object to the end of the list." | ||
gtest(list.append, '(self, object, /)\n' + append_doc) | ||
gtest(List.append, '(self, object, /)\n' + append_doc) | ||
gtest([].append, '(object, /)\n' + append_doc) | ||
|
||
gtest(types.MethodType, "method(function, instance)") | ||
gtest(SB(), default_tip) | ||
import re | ||
p = re.compile('') | ||
gtest(re.sub, '''(pattern, repl, string, count=0, flags=0)\nReturn the string obtained by replacing the leftmost | ||
non-overlapping occurrences of the pattern in string by the | ||
replacement repl. repl can be either a string or a callable; | ||
if a string, backslash escapes in it are processed. If it is | ||
a callable, it's passed the match object and must return''') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would prefer that this be truncated also. We allowed up to 5 docstring lines for builtins to get the signature (bytes needs 5), but if we already have signature, only 1 should be allowed, as below. However, after checking the code and the docstring, I decided that the source of the problem is that the re.sub docstring is defective in not having an overly long summary sentence and then not having a blank line after the first '.', as it should. We could check for '.', but not worth the effort now. Instead I would like to add a comment so it is clear that this case was thought about. This is a good additional test case.
|
||
gtest(p.sub, '''(repl, string, count=0)\nReturn the string obtained by replacing the leftmost non-overlapping occurrences o...''') | ||
|
||
def test_signature_wrap(self): | ||
if textwrap.TextWrapper.__doc__ is not None: | ||
|
@@ -132,12 +143,20 @@ def test_starred_parameter(self): | |
# test that starred first parameter is *not* removed from argspec | ||
class C: | ||
def m1(*args): pass | ||
def m2(**kwds): pass | ||
c = C() | ||
for meth, mtip in ((C.m1, '(*args)'), (c.m1, "(*args)"), | ||
(C.m2, "(**kwds)"), (c.m2, "(**kwds)"),): | ||
for meth, mtip in ((C.m1, '(*args)'), (c.m1, "(*args)"),): | ||
self.assertEqual(signature(meth), mtip) | ||
|
||
def test_invalid_method_signature(self): | ||
class C: | ||
def m2(**kwargs): pass | ||
class Test: | ||
def __call__(*, a): pass | ||
|
||
mtip = ct._invalid_method | ||
self.assertEqual(signature(C().m2), mtip) | ||
self.assertEqual(signature(Test()), mtip) | ||
|
||
def test_non_ascii_name(self): | ||
# test that re works to delete a first parameter name that | ||
# includes non-ascii chars, such as various forms of A. | ||
|
@@ -156,17 +175,23 @@ def test_attribute_exception(self): | |
class NoCall: | ||
def __getattr__(self, name): | ||
raise BaseException | ||
class Call(NoCall): | ||
class CallA(NoCall): | ||
def __call__(oui, a, b, c): | ||
pass | ||
class CallB(NoCall): | ||
def __call__(self, ci): | ||
pass | ||
for meth, mtip in ((NoCall, default_tip), (Call, default_tip), | ||
(NoCall(), ''), (Call(), '(ci)')): | ||
|
||
for meth, mtip in ((NoCall, default_tip), (CallA, default_tip), | ||
(NoCall(), ''), (CallA(), '(a, b, c)'), | ||
(CallB(), '(ci)')): | ||
self.assertEqual(signature(meth), mtip) | ||
|
||
def test_non_callables(self): | ||
for obj in (0, 0.0, '0', b'0', [], {}): | ||
self.assertEqual(signature(obj), '') | ||
|
||
|
||
class Get_entityTest(unittest.TestCase): | ||
def test_bad_entity(self): | ||
self.assertIsNone(ct.get_entity('1/0')) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
IDLE: Calltips use `inspect.signature instead of `inspect.getfullargspec`. | ||
This improves calltips for builtins converted to use Argument Clinic. | ||
Patch by Louie Lu. |
Uh oh!
There was an error while loading. Please reload this page.
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.
Upon seeing a real example, I don't like the clash between () as call operator and comment marker in tips such as
(object, /)
('/' marks preceding arguments as positional-only)
Append object to the end of the list.
I prefer either [] or # as the comment marker. What think you?
(object, /)
['/' marks preceding arguments as positional-only]
Append object to the end of the list.
(object, /)
# '/' marks preceding arguments as positional-only
Append object to the end of the list.
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'll prefer
['/' marks preceding arguments as positional-only]
style, it has more conspicuous when the text was end at the box margin.Also, I think the word is OK, using google to search will got this stackoverflow post at the top, it should not be a problem for newbie.
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.
By 'word', I presume you meant 'positional-only'. Nice check. We'll go with your preference.