From 3969228c4b2cfac52d44e42d92a5cc195c9d1abe Mon Sep 17 00:00:00 2001 From: Louie Lu Date: Tue, 2 May 2017 18:24:55 +0800 Subject: [PATCH 1/8] bpo-19903: Change to inspect.signature for calltips This commit change the get_argspec from using inspect.getfullargspec to inspect.signature. It will improve the tip message for use. Also, if object is not callable, now will return this message for user, not blank tips. If the methods has an invalid method signature, it will also return message to user. --- Lib/idlelib/calltips.py | 32 ++++++++++++----- Lib/idlelib/idle_test/test_calltips.py | 49 +++++++++++++++++++------- 2 files changed, 61 insertions(+), 20 deletions(-) diff --git a/Lib/idlelib/calltips.py b/Lib/idlelib/calltips.py index 4c5aea2acbf40b..0dcc970d451698 100644 --- a/Lib/idlelib/calltips.py +++ b/Lib/idlelib/calltips.py @@ -122,6 +122,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): @@ -138,17 +140,31 @@ def get_argspec(ob): ob_call = ob.__call__ except BaseException: return argspec - if isinstance(ob, type): - fob = ob.__init__ - elif isinstance(ob_call, types.MethodType): + if 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) + + try: + argspec = str(inspect.signature(fob)) + except ValueError as err: + msg = str(err) + if msg.startswith(_invalid_method): + argspec = _invalid_method + return argspec + elif msg.startswith('no signature found for'): + """If no signature found for function or method""" + pass + else: + """Callable is not supported by signature""" + pass + + if '/' in argspec: + """Using AC's positional argument should add the explain""" + argspec = '\n'.join([argspec, _argument_positional]) + 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 []) diff --git a/Lib/idlelib/idle_test/test_calltips.py b/Lib/idlelib/idle_test/test_calltips.py index 29b9f06faf868b..dbbdf14b45b207 100644 --- a/Lib/idlelib/idle_test/test_calltips.py +++ b/Lib/idlelib/idle_test/test_calltips.py @@ -46,6 +46,7 @@ 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 @@ -53,18 +54,28 @@ 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''') + 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')) From b4bb5dc1dff57e4ac18684f4f035504bf59c415b Mon Sep 17 00:00:00 2001 From: Louie Lu Date: Thu, 3 Aug 2017 14:08:57 +0800 Subject: [PATCH 2/8] Add blurb --- Misc/NEWS.d/next/IDLE/2017-08-03-14-08-42.bpo-19903.sqE1FS.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/IDLE/2017-08-03-14-08-42.bpo-19903.sqE1FS.rst diff --git a/Misc/NEWS.d/next/IDLE/2017-08-03-14-08-42.bpo-19903.sqE1FS.rst b/Misc/NEWS.d/next/IDLE/2017-08-03-14-08-42.bpo-19903.sqE1FS.rst new file mode 100644 index 00000000000000..1cbe2853aa7799 --- /dev/null +++ b/Misc/NEWS.d/next/IDLE/2017-08-03-14-08-42.bpo-19903.sqE1FS.rst @@ -0,0 +1 @@ +IDLE: Calltips changed from `inspect.getfullargspec` to `inspect.signature` From 7ba520266ed0d713317256cdbabc76491d03718f Mon Sep 17 00:00:00 2001 From: Louie Lu Date: Thu, 3 Aug 2017 14:13:35 +0800 Subject: [PATCH 3/8] Addressed Terry comments --- Lib/idlelib/calltips.py | 20 +++++--------------- 1 file changed, 5 insertions(+), 15 deletions(-) diff --git a/Lib/idlelib/calltips.py b/Lib/idlelib/calltips.py index 0e792f0d12f470..42b84710cb5b00 100644 --- a/Lib/idlelib/calltips.py +++ b/Lib/idlelib/calltips.py @@ -136,29 +136,19 @@ 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_call, types.MethodType): - fob = ob_call - else: - fob = ob + 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): - argspec = _invalid_method - return argspec - elif msg.startswith('no signature found for'): - """If no signature found for function or method""" - pass - else: - """Callable is not supported by signature""" - pass + return _invalid_method if msg.startswith(_invalid_method) else default if '/' in argspec: """Using AC's positional argument should add the explain""" From a9de23721c8609b46b2049c03fdab723a5ec4671 Mon Sep 17 00:00:00 2001 From: Louie Lu Date: Thu, 3 Aug 2017 14:13:49 +0800 Subject: [PATCH 4/8] Cleanup --- Lib/idlelib/calltips.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Lib/idlelib/calltips.py b/Lib/idlelib/calltips.py index 42b84710cb5b00..1a56ddad875cb1 100644 --- a/Lib/idlelib/calltips.py +++ b/Lib/idlelib/calltips.py @@ -158,7 +158,7 @@ def get_argspec(ob): 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__ @@ -177,6 +177,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) From 39907c72da3c8931aff2d67d910320cead6438a4 Mon Sep 17 00:00:00 2001 From: Louie Lu Date: Thu, 3 Aug 2017 14:19:13 +0800 Subject: [PATCH 5/8] Revert partial code --- Lib/idlelib/calltips.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Lib/idlelib/calltips.py b/Lib/idlelib/calltips.py index 1a56ddad875cb1..588e89e3b9cc77 100644 --- a/Lib/idlelib/calltips.py +++ b/Lib/idlelib/calltips.py @@ -148,7 +148,8 @@ def get_argspec(ob): argspec = str(inspect.signature(fob)) except ValueError as err: msg = str(err) - return _invalid_method if msg.startswith(_invalid_method) else default + if msg.startswith(_invalid_method): + return _invalid_method if '/' in argspec: """Using AC's positional argument should add the explain""" From c495b6142cce25b9e5057a54942637a25d58a2f5 Mon Sep 17 00:00:00 2001 From: Terry Jan Reedy Date: Thu, 3 Aug 2017 21:34:12 -0400 Subject: [PATCH 6/8] Update 2017-08-03-14-08-42.bpo-19903.sqE1FS.rst --- .../NEWS.d/next/IDLE/2017-08-03-14-08-42.bpo-19903.sqE1FS.rst | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/IDLE/2017-08-03-14-08-42.bpo-19903.sqE1FS.rst b/Misc/NEWS.d/next/IDLE/2017-08-03-14-08-42.bpo-19903.sqE1FS.rst index 1cbe2853aa7799..7f9bd2d5968a09 100644 --- a/Misc/NEWS.d/next/IDLE/2017-08-03-14-08-42.bpo-19903.sqE1FS.rst +++ b/Misc/NEWS.d/next/IDLE/2017-08-03-14-08-42.bpo-19903.sqE1FS.rst @@ -1 +1,3 @@ -IDLE: Calltips changed from `inspect.getfullargspec` to `inspect.signature` +IDLE: Calltips use `inspect.signature instead of `inspect.getfullargspec`. +This improves calltips for builtins converted to use Argument Clinic. +Patch by Louie Lu. From 68fccf292bda7c54501f33fcf350218545f2c315 Mon Sep 17 00:00:00 2001 From: Terry Jan Reedy Date: Thu, 3 Aug 2017 23:22:45 -0400 Subject: [PATCH 7/8] Update 2017-08-03-14-08-42.bpo-19903.sqE1FS.rst --- Misc/NEWS.d/next/IDLE/2017-08-03-14-08-42.bpo-19903.sqE1FS.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/IDLE/2017-08-03-14-08-42.bpo-19903.sqE1FS.rst b/Misc/NEWS.d/next/IDLE/2017-08-03-14-08-42.bpo-19903.sqE1FS.rst index 7f9bd2d5968a09..f25fc80c3dd651 100644 --- a/Misc/NEWS.d/next/IDLE/2017-08-03-14-08-42.bpo-19903.sqE1FS.rst +++ b/Misc/NEWS.d/next/IDLE/2017-08-03-14-08-42.bpo-19903.sqE1FS.rst @@ -1,3 +1,3 @@ -IDLE: Calltips use `inspect.signature instead of `inspect.getfullargspec`. +IDLE: Calltips use `inspect.signature` instead of `inspect.getfullargspec`. This improves calltips for builtins converted to use Argument Clinic. Patch by Louie Lu. From 55032cf48cc601863d88c0c4a57b9c9883bbeb17 Mon Sep 17 00:00:00 2001 From: Louie Lu Date: Tue, 8 Aug 2017 12:06:26 +0800 Subject: [PATCH 8/8] Fix _argument_positional words --- Lib/idlelib/calltips.py | 4 ++-- Lib/idlelib/idle_test/test_calltips.py | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/Lib/idlelib/calltips.py b/Lib/idlelib/calltips.py index 588e89e3b9cc77..49625eac158c03 100644 --- a/Lib/idlelib/calltips.py +++ b/Lib/idlelib/calltips.py @@ -124,7 +124,7 @@ def get_entity(expression): _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)" +_argument_positional = "\n['/' marks preceding arguments as positional-only]\n" def get_argspec(ob): @@ -153,7 +153,7 @@ def get_argspec(ob): if '/' in argspec: """Using AC's positional argument should add the explain""" - argspec = '\n'.join([argspec, _argument_positional]) + argspec += _argument_positional if isinstance(fob, type) and argspec == '()': """fob with no argument, use default callable argspec""" argspec = _default_callable_argspec diff --git a/Lib/idlelib/idle_test/test_calltips.py b/Lib/idlelib/idle_test/test_calltips.py index dbbdf14b45b207..fa92ece78ee6b9 100644 --- a/Lib/idlelib/idle_test/test_calltips.py +++ b/Lib/idlelib/idle_test/test_calltips.py @@ -54,17 +54,17 @@ def gtest(obj, out): self.assertEqual(signature(obj), out) if List.__doc__ is not None: - gtest(List, '(iterable=(), /)\n' + ct._argument_positional + '\n' + + gtest(List, '(iterable=(), /)' + ct._argument_positional + '\n' + List.__doc__) gtest(list.__new__, '(*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' + + '(self, /, *args, **kwargs)' + ct._argument_positional + '\n' + 'Initialize self. See help(type(self)) for accurate signature.') - 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) + append_doc = ct._argument_positional + "\nAppend object to the end of the list." + gtest(list.append, '(self, object, /)' + append_doc) + gtest(List.append, '(self, object, /)' + append_doc) + gtest([].append, '(object, /)' + append_doc) gtest(types.MethodType, "method(function, instance)") gtest(SB(), default_tip)