bpo-19903: IDLE: Change to inspect.signature for calltips by louisom · Pull Request #1382 · python/cpython (original) (raw)
This commit change the get_argspec from using inspect.getfullargspec
to inspect.signature. It will improve the tip message for use.
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.
| @@ -136,6 +136,9 @@ def get_argspec(ob): |
|---|
| argspec = "" |
| try: |
| ob_call = ob.__call__ |
| except AttributeError: |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you left this change for separate issue?
| if (isinstance(ob, (type, types.MethodType)) or |
|---|
| isinstance(ob_call, types.MethodType)): |
| argspec = _first_param.sub("", argspec) |
| if argspec.startswith("(self,"): |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if the first parameter have a different name?
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, you are right. I'll still need to figure out how it does different from getfullargspec
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Find out that signature will deal correctly with MethodType, but will not handle __init__ correctly.
Fix the condition of trim first param.
| c = C() |
|---|
| for meth, mtip in ((C.m1, '(*args)'), (c.m1, "(*args)"), |
| (C.m2, "(**kwds)"), (c.m2, "(**kwds)"),): |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove invalid method signature, move to test_invalid_method_signature
| mtip = "This function has an invalid method signature" |
| self.assertEqual(signature(C().m2), mtip) |
| self.assertEqual(signature(Test()), mtip) |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test borrow from test_signature
| @@ -156,17 +164,22 @@ 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): |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add corner case for first param not self
| self.assertEqual(signature(meth), mtip) |
|---|
| def test_non_callables(self): |
| for obj in (0, 0.0, '0', b'0', [], {}): |
| self.assertEqual(signature(obj), '') |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PEP8
| @@ -145,10 +148,16 @@ def get_argspec(ob): |
|---|
| else: |
| fob = ob |
| if isinstance(fob, (types.FunctionType, types.MethodType)): |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove and dedent block.
| argspec = inspect.formatargspec(*inspect.getfullargspec(fob)) |
|---|
| try: |
| argspec = str(inspect.signature(fob)) |
| except ValueError: |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Must get and test error message.
| try: |
|---|
| argspec = str(inspect.signature(fob)) |
| except ValueError: |
| argspec = "This function has an invalid method signature" |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add _invalid_signature to global defs before function so can access for testing without repeating string.
| argspec = inspect.formatargspec(*inspect.getfullargspec(fob)) |
|---|
| try: |
| argspec = str(inspect.signature(fob)) |
| except ValueError: |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Condition return on error message
| except ValueError: |
|---|
| argspec = "This function has an invalid method signature" |
| return argspec |
| if (isinstance(ob, (type, types.MethodType)) or |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add argspec != '' to later revised condition
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See msg292964 on issue for explanations and comment on what test addition is needed.
| @@ -1,4 +1,4 @@ |
|---|
| """calltips.py - An IDLE Extension to Jog Your Memory |
| b"""calltips.py - An IDLE Extension to Jog Your Memory |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
??? Docstrings are not bytes.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A typo..., I'm using trackpoint on laptop, sometime when scrolling down, it will unexpectedly
pressing "b"
| @@ -139,21 +139,32 @@ def get_argspec(ob): |
|---|
| ob_call = ob.__call__ |
| except BaseException: |
| return argspec |
| if isinstance(ob, type): |
| fob = ob.__init__ |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why remove this? It worked before. Does signature() do this automatically? Such a change should be explained on the issue with examples, both for Python and C coded classed. And yes, I wish I had added comments here or in test file or, by reference, an issue, when I revised this and and added test file.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As you said on msg, this will automatically done by signature.
| if '/' in argspec: |
|---|
| """Argument Clinic positional-only mark, ignore the result of signature |
| """ |
| argspec = '' |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why? '/' is used in legitimate signatures, and appear, I believe, in the docs.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bpo-19903: Change to inspect.signature for calltips bpo-19903: IDLE: Change to inspect.signature for calltips
Replaced by #2822 with new account name. LEAVE THIS OPEN FOR NOW. Since it will not be merged, I am using it as one example of not being able to edit the code due to lack permission.
Edit by others being turned off seems to be resolved for now.