[issue25590] tab-completition on instances repeatedly accesses attribute/descriptors values

[Ezio Melotti]

# <tab> is used to indicate when/where I press <tab>

... @property
... def bar(self): print('Foo.bar called')
...

f = Foo()
f.<tab>Foo.bar called

Foo.bar called
Foo.bar called
Foo.bar called
<tab>Foo.bar called
Foo.bar called
Foo.bar called
Foo.bar called
<tab>
f.__class__( f.__doc__ f.__getattribute__( f.__le__( f.__new__( f.__setattr__( f.__weakref__
f.__delattr__( f.__eq__( f.__gt__( f.__lt__( f.__reduce__( f.__sizeof__( f.bar
f.__dict__ f.__format__( f.__hash__( f.__module__ f.__reduce_ex__( f.__str__(
f.__dir__( f.__ge__( f.__init__( f.__ne__( f.__repr__( f.__subclasshook__(

f.b<tab>Foo.bar called

Foo.bar called
Foo.bar called
Foo.bar called
ar<enter>
Foo.bar called

Each time I press <tab>, the property is called 4 times. I'm not sure why the value is accessed at all, but even if there was a valid reason to do so, doing it once should be enough.

----------
messages: 254415
nosy: ezio.melotti
priority: normal
severity: normal
stage: test needed
status: open
title: tab-completition on instances repeatedly accesses attribute/descriptors values
type: behavior
versions: Python 3.4, Python 3.5, Python 3.6

_______________________________________
Python tracker <***@bugs.python.org>
<http://bugs.python.org/issue25590>
_______________________________________

[Martin Panter]

Martin Panter added the comment:

Long story short: it is accessed due to the callable suffix check (Issue 449227), and this check is less than optimal, meaning the attribute gets accessed up to four times per Tab press.

In 3.6, there are only two calls, one for the hasattr() call, and one for the getattr() call:

Foo.bar called
File "/home/proj/python/cpython/Lib/rlcompleter.py", line 85, in complete
self.matches = self.attr_matches(text)
File "/home/proj/python/cpython/Lib/rlcompleter.py", line 164, in attr_matches
hasattr(thisobject, word)):
File "<stdin>", line 3, in bar
Foo.bar called
File "/home/proj/python/cpython/Lib/rlcompleter.py", line 85, in complete
self.matches = self.attr_matches(text)
File "/home/proj/python/cpython/Lib/rlcompleter.py", line 165, in attr_matches
val = getattr(thisobject, word)
File "<stdin>", line 3, in bar

Before revision 4dbb315fe667 (Issue 25011), “words” was a list rather than a set. It gets two “bar” entries, one from dir(f) and one from dir(f.__class__). Maybe it is worth backporting set(), I dunno.

The hasattr() call was added in r65168 (Issue 3396) as a look-before-you-leap check to avoid the completer from dying from getattr() exceptions. IMO in this case it is better to “ask for forgiveness” by catching Exception from getattr(). This would be more robust to quirky and buggy attributes anyway.

Finally (or really, initially), getattr() was added in r64664 (Issue 449227). I think even if getattr() fails, the attribute name could still be output, just not with the callable suffix (which I partly disagree with anyway). It would have been like that previously. But I guess this is debatable, and a separate issue.

----------
components: +Library (Lib)
nosy: +martin.panter, serhiy.storchaka

_______________________________________
Python tracker <***@bugs.python.org>
<http://bugs.python.org/issue25590>
_______________________________________

[Martin Panter]

... @property
... def bar(self): return self._bar
... @bar.setter
... def bar(self, value): self._bar = value
...

f = Foo()
f.bar = ... # Tab completion fails in this case

----------

_______________________________________
Python tracker <***@bugs.python.org>
<http://bugs.python.org/issue25590>
_______________________________________

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Agreed with all your suggestions Martin. Do you want to write a patch?

----------
stage: test needed -> needs patch

_______________________________________
Python tracker <***@bugs.python.org>
<http://bugs.python.org/issue25590>
_______________________________________

[Martin Panter]

Martin Panter added the comment:

getattr-once.patch handles my first two points. It backports the set() change, and avoids the hasattr() check. I guess we apply this to 3.4+; expect merge conflicts with 3.6.

As for the last point, I will make another patch to include special attributes that don’t actually exist yet, where they are currently omitted. Would we consider this a new feature for 3.6? Going by Serhiy’s logic in <https://bugs.python.org/issue25209#msg251514> I would say yes.

----------
keywords: +patch
stage: needs patch -> patch review
Added file: http://bugs.python.org/file41006/getattr-once.patch

_______________________________________
Python tracker <***@bugs.python.org>
<http://bugs.python.org/issue25590>
_______________________________________

[Martin Panter]

Martin Panter added the comment:

uncreated-attr.patch is against the 3.6 branch, and includes the excessive getattr() test, and new code to include uncreated attribute names. I added a paragraph to What’s New describing the new completion behaviour.

----------
Added file: http://bugs.python.org/file41007/uncreated-attr.patch

_______________________________________
Python tracker <***@bugs.python.org>
<http://bugs.python.org/issue25590>
_______________________________________

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Both patches LGTM. Thank you Martin.

Post by Martin Panter
Would we consider this a new feature for 3.6?

We can consider this a fix of the regression in issue449227. But this behavior here is so long. I agree with applying it to 3.6 only.

For nicer Mercurial history, it would be better to apply both patches separately in 3.6.

Would not be simpler to use uninitialized slot attribute in test_uncreated_attr?

----------
assignee: -> martin.panter
stage: patch review -> commit review

_______________________________________
Python tracker <***@bugs.python.org>
<http://bugs.python.org/issue25590>
_______________________________________