Issue12510
This issue tracker has been migrated to GitHub,
and is currently read-only.
For more information,
see the GitHub FAQs in the Python's Developer Guide.
Created on 2011-07-06 23:27 by Roy.Fox, last changed 2022-04-11 14:57 by admin. This issue is now closed.
Files | ||||
---|---|---|---|---|
File name | Uploaded | Description | Edit | |
issue12510.patch | roger.serwy, 2011-12-21 18:37 | review | ||
issue12510_rev1.patch | roger.serwy, 2012-05-27 20:19 | review | ||
fix_12510.patch | roger.serwy, 2012-06-03 21:27 | review | ||
i12510.test.diff | terry.reedy, 2012-06-04 03:01 | Improve get_argspec and tests. | review | |
i12510b.test.diff | terry.reedy, 2012-06-05 09:21 | Improve get_argspec and tests. | ||
i12510c.test.diff | terry.reedy, 2012-06-05 21:06 | Improve get_argspec and tests. | review | |
issue12510_self_pat.patch | roger.serwy, 2012-06-12 19:33 | review |
Messages (29) | |||
---|---|---|---|
msg139961 - (view) | Author: Roy Fox (Roy.Fox) | Date: 2011-07-06 23:27 | |
Hi, When you type (not copy-paste!) into an IDLE shell a string literal followed by ( you get a calltip. When the string contains a bad unicode escaping you get an error (see example below), which makes some sense. But when the string is raw, it isn't treated as such, and you may get the same error, though now it doesn't make any sense. Non-raw example: >>> '\xyz'( >>> *** Internal Error: rpc.py:SocketIO.localcall() Object: exec Method: <bound method Executive.get_the_calltip of <idlelib.run.Executive object at 0x021C2910>> Args: ("'\\xyz'",) Traceback (most recent call last): File "C:\Python32\lib\idlelib\rpc.py", line 188, in localcall ret = method(*args, **kwargs) File "C:\Python32\lib\idlelib\run.py", line 324, in get_the_calltip return self.calltip.fetch_tip(name) File "C:\Python32\lib\idlelib\CallTips.py", line 103, in fetch_tip entity = self.get_entity(name) File "C:\Python32\lib\idlelib\CallTips.py", line 112, in get_entity return eval(name, namespace) File "<string>", line 1 SyntaxError: (unicode error) 'unicodeescape' codec can't decode bytes in position 0-2: truncated \xXX escape Raw example: >>> r'\xyz'( >>> *** Internal Error: rpc.py:SocketIO.localcall() Object: exec Method: <bound method Executive.get_the_calltip of <idlelib.run.Executive object at 0x021C2910>> Args: ("'\\xyz'",) Traceback (most recent call last): File "C:\Python32\lib\idlelib\rpc.py", line 188, in localcall ret = method(*args, **kwargs) File "C:\Python32\lib\idlelib\run.py", line 324, in get_the_calltip return self.calltip.fetch_tip(name) File "C:\Python32\lib\idlelib\CallTips.py", line 103, in fetch_tip entity = self.get_entity(name) File "C:\Python32\lib\idlelib\CallTips.py", line 112, in get_entity return eval(name, namespace) File "<string>", line 1 SyntaxError: (unicode error) 'unicodeescape' codec can't decode bytes in position 0-2: truncated \xXX escape |
|||
msg140051 - (view) | Author: Terry J. Reedy (terry.reedy) * ![]() |
Date: 2011-07-09 05:42 | |
What platform? It sometimes makes a difference with tcl/tk and hence idle. |
|||
msg140056 - (view) | Author: Ned Deily (ned.deily) * ![]() |
Date: 2011-07-09 09:31 | |
The problem is easily reproducible. Although it shouldn't give that error (and that can be fixed), it seems to me that IDLE should not be trying to give a calltip in that context. What it is trying to do is display the __doc__ attribute of the string but the __doc__ is really for the str() constructor: >>> 'a'.__doc__ "str(string[, encoding[, errors]]) -> str\n\nCreate a new string object from the given encoded string.\nencoding defaults to the current default string encoding.\nerrors can be 'strict', 'replace' or 'ignore' and defaults to 'strict'." |
|||
msg150033 - (view) | Author: Roger Serwy (roger.serwy) * ![]() |
Date: 2011-12-21 18:37 | |
The attached patch fixes the bug. The bug occurs in "get_entity" which is used to get the object. Then "get_argspec" determines the calltip text. The calltip can be prevented for strings by having get_argspec check if the object has a "__call__" method. I opted for adding "not callable" to the calltip initially for testing, but setting argspec="" will prevent the calltip altogether. |
|||
msg161698 - (view) | Author: Terry J. Reedy (terry.reedy) * ![]() |
Date: 2012-05-27 07:31 | |
With 2.7 & 3.3 on Win 7, 'a'( brings up calltip (bad). With 3.3, '\xyz'( causes Idle to die (worse) With 2.7, the same or u'\xyz'( have no effect, just like 3(. There are a lot of little differences between 2.7 and 3.x, but the main one is the use of inspect in 3.x. I do not see an obvious reason for the behavior difference. With patch, 'a'( brings up calltip with 'Not callable' added (worse, in my opinion), while '\xyz'( does nothing, just like 3( (or 2.7, great). Roger, perhaps you uploaded your initial test past instead of the final patch. A string instance should act like any other non-callable instance. That said, the patch strikes me as attacking the problem at the wrong end. Str instances are not names and get_argspec should never be called on one (I am presuming that it is not called on other non-callables, such as int instances). Similarly, back up further in the call stack, fetch_tip(name) should only be called on names. I wonder if the problem might be back up in open_calltip(), perhaps after hp = HyperParser(self.editwin, "insert") sur_paren = hp.get_surrounding_brackets('(') if not sur_paren: return hp.set_index(sur_paren[0]) name = hp.get_expression() (argggh! I have no idea how to debug code where I cannot just insert print statements to tell me what is getting called with what arguments.) |
|||
msg161721 - (view) | Author: Roger Serwy (roger.serwy) * ![]() |
Date: 2012-05-27 20:19 | |
Terry, the original patch added "Not callable" because I wasn't sure what the consensus was on proper behavior. Now I know. :) Attached is a revision against 3.3a3+ which omits the calltip if the object is not callable. The behavior difference between 2.7 and 3.3 is due to a difference in "localcall" in rpc.py, where "CALLEXC" raises the exception in the IDLE front-end in 3.3 causing the crash, whereas in 2.7 the exception occurs in the subprocess. This is due to "rpc_marshal_exception.patch" for issue14200 not being backported to 2.7. Entering []( or {}( or ''( will bring up a calltip. These calltips should not appear if the object is not callable. Take as an example: a = "string" a( The previous code will bring up a calltip even though "a" is not callable. The HyperParser does its job by returning the string representing the object just before the paren. Checking whether this is a proper Python name won't fix this corner case. The "fetch_tip" method grabs the actual object given the string name using "get_entity" and then uses "get_argspec" to return constructor or callable arguments. Checking if the object is callable must be done either in "get_entity" or in "get_argspec". I think it is cleaner to check in get_argspec, as is done in the attached patch. |
|||
msg161742 - (view) | Author: Terry J. Reedy (terry.reedy) * ![]() |
Date: 2012-05-28 00:10 | |
'a'(, [](, {}( no longer do anything. [int][0]( and {0:int}[0] bring up int tooltip. However, patch is not a complete fix yet. [int][1]( and {0:int}[1] both 'crash' Idle. So we need to add IndexError and KeyError to + except (NameError, AttributeError, SyntaxError): I think instead we should just remove the list since any uncaught error in the eval will close Idle. Do you see any reason why not? If open_calltips were called with evalfuncs True (I presume there must be a way, see below), then *any* exception could happen in the eval. I was thinking that eval is a bit dangerous but I see this protection in open_calltips" if not evalfuncs and (name.find('(') != -1): return Consequently, after "def f(): return int", f()( does not get a calltip. Neither does the tuple equivalent of the list example above, (int,)[0](. With the call check moved from the bottom of get_argspec to the top, I am ok with it there. If we agree on the except clause, I am ready to apply. |
|||
msg161743 - (view) | Author: Roger Serwy (roger.serwy) * ![]() |
Date: 2012-05-28 00:20 | |
I'm ok with removing the exception list, but add a comment like "# *any* exception could happen in the eval". |
|||
msg161748 - (view) | Author: Roundup Robot (python-dev) ![]() |
Date: 2012-05-28 01:55 | |
New changeset 938b12452af7 by Terry Jan Reedy in branch '2.7': Issue12510: Attempting to get invalid tooltip no longer closes Idle. http://hg.python.org/cpython/rev/938b12452af7 New changeset 4a7582866735 by Terry Jan Reedy in branch '3.2': Issue12510: Attempting to get invalid tooltip no longer closes Idle. http://hg.python.org/cpython/rev/4a7582866735 New changeset 0835bee19f86 by Terry Jan Reedy in branch 'default': Merge 3.2 closes #12510 http://hg.python.org/cpython/rev/0835bee19f86 |
|||
msg161828 - (view) | Author: Terry J. Reedy (terry.reedy) * ![]() |
Date: 2012-05-29 01:43 | |
I am reopening just as a reminder to revise the comment. |
|||
msg161847 - (view) | Author: Roger Serwy (roger.serwy) * ![]() |
Date: 2012-05-29 03:36 | |
Which comment needs revision? |
|||
msg162192 - (view) | Author: Roundup Robot (python-dev) ![]() |
Date: 2012-06-03 05:07 | |
New changeset 477508efe4ab by Terry Jan Reedy in branch '2.7': Issue 12510: Expand 2 bare excepts. Improve comments. Change deceptive name http://hg.python.org/cpython/rev/477508efe4ab New changeset f927a5c6e4be by Terry Jan Reedy in branch '3.2': Issue 12510: Expand 2 bare excepts. Improve comments. Change deceptive name http://hg.python.org/cpython/rev/f927a5c6e4be New changeset a7501ddf74ac by Terry Jan Reedy in branch 'default': Merge with 3.2 #12510 http://hg.python.org/cpython/rev/a7501ddf74ac |
|||
msg162195 - (view) | Author: Terry J. Reedy (terry.reedy) * ![]() |
Date: 2012-06-03 05:17 | |
CallTips.py has a 'main' test at the end. Currently, test TC fails in 3.x but not 2.7. So either the test became invalid or get_argspec was not completely and correctly converted from get_arg_text. This should be fixed. int.append( does not bring up a tip on either version, but should if possible. The failing examples in previous messages should be added to the tests to ensure that all pass now and continue to pass in the future. So I am not closing this yet, and a new patch is needed. |
|||
msg162233 - (view) | Author: Roger Serwy (roger.serwy) * ![]() |
Date: 2012-06-03 21:27 | |
fix_12510.patch addresses the issue with the test. What do you mean by: "int.append( does not bring up a tip on either version, but should if possible." ? The "int" object does not have "append" as a method. |
|||
msg162248 - (view) | Author: Terry J. Reedy (terry.reedy) * ![]() |
Date: 2012-06-04 03:01 | |
i12510.test.diff (for Python 3 only) does the following: * Turn method CallTips.get_entity into a module function as it does not use the self parameter and is therefore not really an instance method. * Delete the erroneous _find_constructor function. Even if it were fixed, it is unnecessary at least in 3.x. Class attribute lookup already does the superclass search duplicated in this function and should find object.__init__ if nothing else. (I defaulted to None anyway in case of an unforeseen problem, such as a type subclass that somehow disables the lookup.) * In get_argspec, remove the lambda stuff that resulted in all classes getting a spurious '()' line. With the fix to retrieving .__init__, it never gets used and is not needed anyway. * Only delete 'self' from the argument list for classes and bound methods where it is implicit and not to be explicity entered. * Increase tests from 13 to 37. * Modernize and re-organize tests with an eye to becoming part of an idlelib test suite. With the increased number of tests, I am pretty confident in the patch, but I would not mind if you glance at it and try it out on Linux. |
|||
msg162324 - (view) | Author: Roger Serwy (roger.serwy) * ![]() |
Date: 2012-06-05 03:09 | |
The reoganization in i12510.test.diff will make back-porting to 2.7 slightly more difficult since old and new style classes exist. I do agree with your reasoning for re-factoring the code. (Also, as an extremely minor point, running reindent.py adjusts "pos = 70".) I applied the patch and it works under 11.04 Ubuntu. All the tests pass and the behavior of the CallTip gives reasonable results when used interactively. I did manage to find a corner-case where a CallTip should arise: >>> class A: def __init__(self, a=None): print('init') def __call__(self, b=None): print('call') >>> c = A( Gives "(a=None)" as the call-tip. However, once the object is created, giving the __call__ argument doesn't work, i.e. >>> c( doesn't give a call-tip. This behavior is also the same without the patch. I'll keep playing with the patch to see what else needs improving with CallTips. We might as well fix it completely. |
|||
msg162334 - (view) | Author: Terry J. Reedy (terry.reedy) * ![]() |
Date: 2012-06-05 09:21 | |
By 'Python 3 only', I meant that I am not personally planning to backport to 2.7. I do not want to edit the more complicated get_arg_text and consider other issues higher priority. I meant to include a test case for callable instances, but forgot. When I add to test class TC " def __call__(self, ci): "(self, ci)" tc( gives the inherited TC.__doc__ as the tool tip, which in this case happens to by the artificial "(ai=None, *b)". Adding the following to get_argspec: # after current if is...type: fob = ... elif isinstance(ob.__call__, types.MethodType): fob = ob.__call__ & or isinstance(ob.__call__, types.MethodType) # to self-strip conditional, as instance is already typed & if isinstance(ob.__call__, types.MethodType): doc = ob.__call__.__doc__ else: # before current doc = line, which gets indented makes this new test pass. fdoc = tc.__call__.__doc__ test('tc', fdoc + "\n" + fdoc) Revised patch with above attached. Note 1: Programming is more fun with decent tests. Note 2: These non-visual automatic test do not completely substitute for visual testing with the gui. When I first used a broader conditional than the 'elif' above, one that caught list.append and [].append, the tests passed, but typing list.append( resulting in weird behavior: something flashed and the typed text moved down a line. |
|||
msg162335 - (view) | Author: Terry J. Reedy (terry.reedy) * ![]() |
Date: 2012-06-05 09:40 | |
Stephen D'Aprano just asked on Python list how to get signatures of builtins, noting that calling inspect.getargspec on a Python-coded subclass of a builtin class raises an exception. So we need another testcase, verify the behavior, and probably add try-except around the inspect call. (Perhaps inspect.py should be revised to not leak an exception when given the advertised acceptible input, but that is another issue.) |
|||
msg162336 - (view) | Author: Terry J. Reedy (terry.reedy) * ![]() |
Date: 2012-06-05 09:55 | |
Upon further thought, not a problem. Stephen considered the case: class myint(int): # inherit __init__ def added_func(self): "some new function of an int" In this case, fob = myint.__init__ would not be FunctionType and inspect would not be called. Tool tip would be myint.__doc__ and writer could follow style of builtins. Still an interesting test case to add. A different question is whether the default for callables (as opposed to non-callables) should be nothing or something like 'args unknown' or 'see docs'. |
|||
msg162353 - (view) | Author: Roger Serwy (roger.serwy) * ![]() |
Date: 2012-06-05 16:03 | |
Callable instances now return a call tip. Good! I agree with your analysis of the issue raised by Stephen. The get_argspec function will not fail unexpectedly. > but typing list.append( resulting in weird behavior: something flashed and the typed text moved down a line. I get "L.append(object) -> None -- append object to end" as the call tip on Linux. I'm not sure why it didn't work on Win 7. Could an unintentional enter key-press be the culprit? > A different question is whether the default for callables (as opposed to non-callables) should be nothing or something like 'args unknown' or 'see docs'. I agree that if the argspec string has no contents after all the code in get_argspec, then it should have a message added. Perhaps the following would be sufficient: if hasattr(ob, '__call__'): [truncated] if not argspec: argspec += "Arguments unknown. See docs." return argspec |
|||
msg162373 - (view) | Author: Terry J. Reedy (terry.reedy) * ![]() |
Date: 2012-06-05 21:06 | |
The weird behavior was as if the cursor were moved to the beginning of the line and <Enter> pressed. I cannot duplicate it. I had elif not isinstance(ob, (types.FunctionType, types.MethodType)): instead elif isinstance(ob.__call__, types.MethodType): but something else must have been different, as list.append( works now with above changed back. The tests pass because passing fob to inspect is guarded. But the correct conditional is needed anyway elsewhere in the file. So I will forget this until it happens again ;-). Since we (intend to) inspect all Python-coded callables, and since that returns at least '()' for no args (there is already a test for that), empty callable argspec should only happen for builtin and extension callables with no docstring. I added this code: _default_callable_argspec = "No docstring, see docs." # before get_argspec if not argspec: argspec = _default_callable_argspec I added tests for this and for subclasses of builtins and uploaded a new patch. Can you think of anything else to test? Now that the code and test cases seem about settled, the other (big) change I would like to make is to make most of the tests into proper unittests of get_argspec (which means passing an object, not a string) and some (with some overlap) into unittests of get_entity (passing expression string, getting object). Since rpcclt is None when tests call CallTips.fetch_tip, the method reduces to get_argspec(get_entity(expression)), which just mixes tests of the two functions together. |
|||
msg162510 - (view) | Author: Roundup Robot (python-dev) ![]() |
Date: 2012-06-08 00:05 | |
New changeset 02b4c62ce393 by Terry Jan Reedy in branch '3.2': Issue #12510: Revise and triple # of calltip tests, with an eye to unittest http://hg.python.org/cpython/rev/02b4c62ce393 New changeset 03b5f75ddac7 by Terry Jan Reedy in branch 'default': Merge from 3.2, #12510 http://hg.python.org/cpython/rev/03b5f75ddac7 |
|||
msg162511 - (view) | Author: Terry J. Reedy (terry.reedy) * ![]() |
Date: 2012-06-08 00:14 | |
I decided to commit the current patch (i12510c.test.diff + NEWS changes) as is. Another change is needed to correctly delete the first arg of all methods, not just instance methods with the first arg named 'self'. (In the current new classmethod test, I just allowed non-removal of 'cls'. There should also be a test with a variant of 'self'.) In _self_pat = re.compile('self\,?\s*'), 'self' in the pattern should be replaced by '(xxx', where 'xxx' is the pattern that matches an identifier. (And add '(' to the replacement pattern.) If anyone can quickly tell me what that is, that would be helpful. |
|||
msg162690 - (view) | Author: Roger Serwy (roger.serwy) * ![]() |
Date: 2012-06-12 19:33 | |
The _self_pat RE needs to be changed to just remove the first argument. Presently, another bug exists with the current implementation: >>> class A: def t(self, self1, self2): pass >>> a = A() >>> a.t( gives "(1,2)" as the calltip, instead of "(self1, self2)" for 3.x. Python 2.7 gives the correct calltip. The attached patch modifies _self_pat to remove only the first argument, modifies the classmethod test, and adds a test for "notself", as Terry requested in msg162511. |
|||
msg165057 - (view) | Author: Roundup Robot (python-dev) ![]() |
Date: 2012-07-09 04:15 | |
New changeset eea379307efa by Terry Jan Reedy in branch '3.2': Issue 12510: Delete actual first param name for all methods; revise tests. http://hg.python.org/cpython/rev/eea379307efa New changeset 464c6a50b0ce by Terry Jan Reedy in branch 'default': Merge with 3.2 Issue 12510 http://hg.python.org/cpython/rev/464c6a50b0ce |
|||
msg165058 - (view) | Author: Terry J. Reedy (terry.reedy) * ![]() |
Date: 2012-07-09 04:17 | |
Thank you for the 'positive lookbehind assertion' for anchoring the match after '('. That is what I was missing. '[^,]' is too broad as, in particular, it matches the ')' in 'a(self)'. We don't want the ')' removed. The tests passed with a faulty re because the method test was faulty in that it only tested that something is removed, but not that the removal is correct. I fixed the tests also to really test proper removal. |
|||
msg165793 - (view) | Author: Roger Serwy (roger.serwy) * ![]() |
Date: 2012-07-18 19:40 | |
Are there any other aspects of the calltip handler that requires fixing? If not, then I can start back-porting the patches. |
|||
msg165822 - (view) | Author: Terry J. Reedy (terry.reedy) * ![]() |
Date: 2012-07-19 01:16 | |
The original patches were to all 3 versions. The later patches were to both 3.x versions and mostly involved get_argspec and expanding the tests. Get_argspec is somewhat different from get_arg_text, the 2.x function it replaced. The main difference I could see is that the latter attempt to deal with old-style classes (I think) and does not use inspect. I *think* I am done changing get_argspec until I try making use of the new PEP 362 function signature objects, but that can only affect 3.3+. (This would be to improve calltips for builtins). So if you want to backport further changes to 2.7, so it can pass more of the expanded test suite, now is a good time. Since the TC test class in 2.7 is derived from object, there seems to be no test of old style classes, so I don't know if just replacing get_arg_text with get_argspec would break anything that actually works, assuming that get_argspec does work ;-). |
|||
msg177361 - (view) | Author: Roger Serwy (roger.serwy) * ![]() |
Date: 2012-12-12 01:33 | |
I haven't had any serious problems with calltips on the dev branches for 2.7 and 3.4, except for issue16630. I think this issue can be closed as being completed in order to avoid further expanding its scope. Feel free to reopen if anyone disagrees. |
History | |||
---|---|---|---|
Date | User | Action | Args |
2022-04-11 14:57:19 | admin | set | github: 56719 |
2012-12-12 01:33:41 | roger.serwy | set | status: open -> closed resolution: fixed messages: + msg177361 stage: patch review -> resolved |
2012-07-19 01:16:10 | terry.reedy | set | messages: + msg165822 |
2012-07-18 19:40:08 | roger.serwy | set | messages: + msg165793 |
2012-07-09 04:17:31 | terry.reedy | set | messages: + msg165058 |
2012-07-09 04:15:58 | python-dev | set | messages: + msg165057 |
2012-06-12 19:33:52 | roger.serwy | set | files:
+ issue12510_self_pat.patch messages: + msg162690 |
2012-06-08 00:14:28 | terry.reedy | set | messages: + msg162511 |
2012-06-08 00:05:18 | python-dev | set | messages: + msg162510 |
2012-06-05 21:06:52 | terry.reedy | set | files:
+ i12510c.test.diff messages: + msg162373 |
2012-06-05 16:03:34 | roger.serwy | set | messages: + msg162353 |
2012-06-05 09:55:43 | terry.reedy | set | messages: + msg162336 |
2012-06-05 09:40:57 | terry.reedy | set | messages: + msg162335 |
2012-06-05 09:21:08 | terry.reedy | set | files:
+ i12510b.test.diff messages: + msg162334 |
2012-06-05 03:09:20 | roger.serwy | set | messages: + msg162324 |
2012-06-04 03:01:53 | terry.reedy | set | files:
+ i12510.test.diff messages: + msg162248 stage: needs patch -> patch review |
2012-06-03 21:27:22 | roger.serwy | set | files:
+ fix_12510.patch messages: + msg162233 |
2012-06-03 05:17:14 | terry.reedy | set | resolution: fixed -> (no value) title: IDLE get_the_calltip mishandles raw strings -> IDLE: calltips mishandle raw strings and other examples messages: + msg162195 stage: resolved -> needs patch |
2012-06-03 05:07:31 | python-dev | set | messages: + msg162192 |
2012-05-29 03:36:10 | roger.serwy | set | messages: + msg161847 |
2012-05-29 01:43:40 | terry.reedy | set | status: closed -> open messages: + msg161828 |
2012-05-28 03:14:40 | terry.reedy | set | assignee: terry.reedy |
2012-05-28 01:55:03 | python-dev | set | status: open -> closed nosy: + python-dev messages: + msg161748 resolution: fixed stage: commit review -> resolved |
2012-05-28 00:20:54 | roger.serwy | set | messages: + msg161743 |
2012-05-28 00:10:58 | terry.reedy | set | stage: commit review |
2012-05-28 00:10:44 | terry.reedy | set | messages: + msg161742 |
2012-05-27 20:19:23 | roger.serwy | set | files:
+ issue12510_rev1.patch messages: + msg161721 |
2012-05-27 07:31:12 | terry.reedy | set | messages:
+ msg161698 versions: + Python 2.7, Python 3.3 |
2011-12-21 18:37:53 | roger.serwy | set | files:
+ issue12510.patch nosy: + roger.serwy messages: + msg150033 keywords: + patch |
2011-07-09 09:31:41 | ned.deily | set | nosy:
+ ned.deily messages: + msg140056 |
2011-07-09 05:42:15 | terry.reedy | set | nosy:
+ terry.reedy messages: + msg140051 |
2011-07-06 23:27:21 | Roy.Fox | create |