py: Support object getattr, delattr and setattr #2689

dmazzella · 2016-12-15T16:40:11Z

This patch implements methods __delattr__, __getattr__ and __setattr__ for user-type and allows the use of calls like super(A, self).__setattr__('a', 1)

for discussion see #2375

pfalcon · 2016-12-15T18:06:22Z

py/objtype.c

@@ -533,6 +533,15 @@ STATIC void mp_obj_instance_load_attr(mp_obj_t self_in, qstr attr, mp_obj_t *des

    // try __getattr__
    if (attr != MP_QSTR___getattr__) {
+


This empty line is superfluous.

pfalcon · 2016-12-15T18:06:41Z

py/objtype.c

@@ -533,6 +533,15 @@ STATIC void mp_obj_instance_load_attr(mp_obj_t self_in, qstr attr, mp_obj_t *des

    // try __getattr__
    if (attr != MP_QSTR___getattr__) {
+
+        #if MICROPY_PY_ATTRS_METHODS
+        // if __getattr__ was called with attr == __setattr__  return MP_OBJ_NULL for indicate success


Typo in comment

pfalcon · 2016-12-15T18:16:20Z

Commit message should explain how all this stuff works.

pfalcon · 2016-12-15T18:17:00Z

This change isn't exactly compatible with Python3:

$ python3
Python 3.4.3 (default, Nov 17 2016, 01:08:31) 
[GCC 4.8.4] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> object().__detattr__
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: 'object' object has no attribute '__detattr__'
>>> object().__getattr__
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: 'object' object has no attribute '__getattr__'

dhylands · 2016-12-15T20:12:15Z

@pfalcon I don't understand your comment. If I run the unix build of micropython using this PR and your example:

650 >./micropython 
MicroPython v1.8.6-200-g0db9a7f-dirty on 2016-12-15; linux version
Use Ctrl-D to exit, Ctrl-E for paste mode
>>> object().__detattr__
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: 'object' object has no attribute '__detattr__'
>>> object().__getattr__
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: 'object' object has no attribute '__getattr__'
>>>

I get exactly the same output as python3. Perhaps you could clarify what you meant?

pfalcon · 2016-12-15T20:27:19Z

tests/basics/delgetsetattr.py

+    del a.a
+    print(a.a)
+except AttributeError:
+    print("AttributeError")


Missing newline.

pfalcon · 2016-12-15T20:31:29Z

I get exactly the same output as python3. Perhaps you could clarify what you meant?

Ok, then I'm wrong, I didn't run-test this patch. But something in this patch looks superfluous, maybe this check in if (attr != MP_QSTR___getattr__) is done only for CPython3 compatibility? As I mentioned above, would be nice to have detailed commit message how this all stuff works together.

dmazzella · 2016-12-15T20:54:46Z

Ok, then I'm wrong, I didn't run-test this patch. But something in this patch looks superfluous, maybe this check in if (attr != MP_QSTR___getattr__) is done only for CPython3 compatibility? As I mentioned above, would be nice to have detailed commit message how this all stuff works together.

@pfalcon if (attr != MP_QSTR___getattr__) It existed before my patch.

dmazzella · 2016-12-15T21:06:10Z

@pfalcon @dhylands the unix version of this patch has the macro "MICROPY_PY_ATTRS_METHODS" setted to 0 inherited from py/mpconfig.h

only into "coverage" version the macro "MICROPY_PY_ATTRS_METHODS" is enabled.

Enabling it:

dmazzella@bugged-pc MSYS ~/micropython_dmazzella/unix
$ ./micropython.exe
MicroPython v1.8.6-200-g0db9a7f-dirty on 2016-12-15; linux version
Use Ctrl-D to exit, Ctrl-E for paste mode
>>> object().__getattr__
<bound_method ffdf6a40 <object>.<function>>
>>>
>>>

dmazzella@bugged-pc MSYS ~/micropython_dmazzella/unix
$ python3
Python 3.4.3 (default, Mar  4 2016, 10:23:36)
[GCC 4.9.2] on msys
Type "help", "copyright", "credits" or "license" for more information.
>>> object().__getattr__
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: 'object' object has no attribute '__getattr__'
>>>

dhylands · 2016-12-15T21:22:55Z

ok - I was confusing config options. When I enable it properly, then object().__getattr__ exists, when it shouldn't.

__getattr__ is only supposed to be called, if it exists, when an attribute lookup fails. So the only sensible functionality that should exist in object().__getattr__ is to throw an exception, and that should be in the attribute lookup logic (i.e. if an attribute lookup fails and __getattr__ doesn't exist, then it should throw an exception).

object().__getattribute__ on the other hand, should exist, and would be the logical place to have the logic that's currently in object().__getattr__ (i.e. __getattribute__ should be the function that does the lookup, and if the attrbiute doesn't exist, then call __getattr__ if it exists).

pfalcon · 2016-12-15T21:31:03Z

@dmazzella : Ok. My note on different behavior regarding __*attr__ lookup on object() was just that - a note. It's not deciding (well, not so far). Please update commit message with description how this patch works.

pfalcon · 2016-12-15T21:31:48Z

(Note there's no need to close one pullreq and open another if you update a patch - just push --force after rebase/amend).

dmazzella · 2016-12-16T09:55:04Z

@pfalcon

C:\Users\Damiano>c:\Python3.4\python.exe
Python 3.4.3 (v3.4.3:9b73f1c3e601, Feb 24 2015, 22:43:06) [MSC v.1600 32 bit (Intel)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> object().__delattr__
<method-wrapper '__delattr__' of object object at 0x00DD14B8>
>>> object().__getattr__
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: 'object' object has no attribute '__getattr__'
>>> object().__setattr__
<method-wrapper '__setattr__' of object object at 0x00DD14B0>
>>>

dma@dma-pc MSYS ~/micropython_dmazzella/unix
$ python3 -B
Python 3.4.3 (default, Mar  4 2016, 10:23:36)
[GCC 4.9.2] on msys
Type "help", "copyright", "credits" or "license" for more information.
>>> object().__delattr__
<method-wrapper '__delattr__' of object object at 0x6ffffdc2070>
>>> object().__getattr__
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: 'object' object has no attribute '__getattr__'
>>> object().__setattr__
<method-wrapper '__setattr__' of object object at 0x6ffffdc2080>
>>>

only __getattr__ raise AttributeError

dma@dma-pc MSYS ~/micropython_dmazzella/unix
$ ./micropython
MicroPython v1.8.6-200-g0db9a7f-dirty on 2016-12-16; linux version
Use Ctrl-D to exit, Ctrl-E for paste mode
>>> object().__delattr__
<bound_method ffdf6860 <object>.<function>>
>>> object().__getattr__
<bound_method ffdf6920 <object>.<function>>
>>> object().__setattr__
<bound_method ffdf6a00 <object>.<function>>
>>>

dmazzella · 2016-12-16T10:16:52Z

From Python3 docs the correct way to implement attribute access is implement method __getattribute__ ref @dhylands comment.

@pfalcon and @dpgeorge i have implemented __getattribute__ please give me feedback on this.

…etattr__ implemented __getattribute__

dmazzella · 2016-12-16T14:20:59Z

@pfalcon now we can see:

dma@dma-pc MSYS ~/micropython_dmazzella/unix
$ ./micropython
MicroPython v1.8.6-200-g0db9a7f-dirty on 2016-12-16; linux version
Use Ctrl-D to exit, Ctrl-E for paste mode
>>> object().__delattr__
<bound_method ffdf6940 <object>.<function>>
>>> object().__getattribute__
<bound_method ffdf6a20 <object>.<function>>
>>> object().__setattr__
<bound_method ffdf6b00 <object>.<function>>
>>> object().__getattr__
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: 'object' object has no attribute '__getattr__'
>>>

dma@dma-pc MSYS ~/micropython_dmazzella/unix
$ python3 -B
Python 3.4.3 (default, Mar  4 2016, 10:23:36)
[GCC 4.9.2] on msys
Type "help", "copyright", "credits" or "license" for more information.
>>> object().__delattr__
<method-wrapper '__delattr__' of object object at 0x6ffffdc2070>
>>> object().__getattribute__
<method-wrapper '__getattribute__' of object object at 0x6ffffdc2080>
>>> object().__setattr__
<method-wrapper '__setattr__' of object object at 0x6ffffdc2070>
>>> object().__getattr__
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: 'object' object has no attribute '__getattr__'
>>>

dmazzella · 2016-12-21T11:18:59Z

I have measured performance on pyboard 1.1 with this script:

# test __delattr__ , __getattribute__, __getattr__ and __setattr__
import pyb

class A:
    def __init__(self, d):
        self.d = d

    def __getattribute__(self, attr):
        return super(A, self).__getattribute__(attr)

    def __getattr__(self, attr):
        if  attr in self.d:
            return self.d[attr]
        else:
            raise AttributeError(attr)

    def __setattr__(self, attr, value):
        super(A, self).__setattr__(attr, value)

    def __delattr__(self, attr):
        del self.d[attr]

def main():
    start = pyb.micros()
    a = A({'a':1, 'b':2})
    print(pyb.elapsed_micros(start))
    start = pyb.micros()
    #print(a.a, a.b)
    print(pyb.elapsed_micros(start))
    try:
        start = pyb.micros()
        del a.a
        print(pyb.elapsed_micros(start))
        print(a.a)
    except AttributeError:
        print("AttributeError")

Without MICROPY_PY_ATTRS_METHODS:

MicroPython v1.8.6-201-gbb688b2-dirty on 2016-12-21; PYBv1.1 with STM32F405RG
Type "help()" for more information.
>>> import delgetsetattr
>>> delgetsetattr.main()
53
19
AttributeError
>>>

with MICROPY_PY_ATTRS_METHODS:

MicroPython v1.8.6-201-gbb688b2-dirty on 2016-12-21; PYBv1.1 with STM32F405RG
Type "help()" for more information.
>>> import delgetsetattr
>>> delgetsetattr.main()
77
19
41
AttributeError
>>>

dmazzella · 2016-12-21T11:28:17Z

I have measured performance on pyboard 1.1 with this script:

# test __getattr__
import pyb

class A:
    def __init__(self, d):
        self.d = d

    def __getattr__(self, attr):
        return self.d[attr]

def main():
    start = pyb.micros()
    a = A({'a':1, 'b':2})
    #print(a.a, a.b)
    print(pyb.elapsed_micros(start))

Without MICROPY_PY_ATTRS_METHODS:

MicroPython v1.8.6-201-gbb688b2-dirty on 2016-12-21; PYBv1.1 with STM32F405RG
Type "help()" for more information.
>>> import getattr
>>> getattr.main()
53
>>>

With MICROPY_PY_ATTRS_METHODS:

MicroPython v1.8.6-201-gbb688b2-dirty on 2016-12-21; PYBv1.1 with STM32F405RG
Type "help()" for more information.
>>> import getattr
>>> getattr.main()
56
>>>

dmazzella · 2016-12-21T11:30:09Z

I have measured performance on pyboard 1.1 with this script:

import pyb

class A:

    var = 132

    def __init__(self):
        self.var2 = 34

    def meth(self, i):
        return 42 + i


def main():
    start = pyb.micros()
    a = A()
    getattr(a, "var")
    getattr(a, "var2")
    getattr(a, "meth")(5)
    getattr(a, "_none_such", 123)
    getattr(list, "foo", 456)
    getattr(a, "va" + "r2")
    print(pyb.elapsed_micros(start))

Without MICROPY_PY_ATTRS_METHODS:

MicroPython v1.8.6-201-gbb688b2-dirty on 2016-12-21; PYBv1.1 with STM32F405RG
Type "help()" for more information.
>>> import getattr1
>>> getattr1.main()
168
>>>

With MICROPY_PY_ATTRS_METHODS:

MicroPython v1.8.6-201-gbb688b2-dirty on 2016-12-21; PYBv1.1 with STM32F405RG
Type "help()" for more information.
>>> import getattr1
>>> getattr1.main()
157
>>>

dmazzella · 2016-12-28T09:49:29Z

@dpgeorge @pfalcon any suggestions for improvement?

dpgeorge · 2016-12-29T04:59:37Z

@dmazella an important performance test is for code that doesn't use this new feature. Eg just a simple load of an instance member, like this:

class A:
    def __init__(self):
        self.a = 1
    def test_set(self):
        self.a = 1
        self.a = 1
        self.a = 1
        self.a = 1

And then benchmark the a.test_get() method 100's of times in a loop, measuring its speed. Do it with and without your patch. You can also do the same with get and del (but they probably won't change much, I'm guessing).

dpgeorge · 2016-12-29T05:05:27Z

You will need to write tests to cover all the new lines of code that you add. There are 10 new lines in objobject.c that you don't hit with your test file. Because this is tricky stuff it's important to have test for the behaviour, to match CPython.

Also, it's not clear to me that the additional methods for object are needed: surely __setattr__ and __delattr_ can work correctly without the methods existing in object? You only need object to implement these methods because your tests use super...

@stinos I recall that you tried out these new special methods. Did you need to use super and require object to have the special methods __setattr__ etc?

stinos · 2016-12-29T08:44:13Z

@dpgeorge I used this code so no need to call super. Derives from dict so object might be implicit. This was almost a month ago though with a different version of the code so I don't know how it behaves currently and I cannot test it at the time of writing, this afternoon maybe.

pfalcon · 2016-12-29T08:59:15Z

@pfalcon any suggestions for improvement?

I already gave my suggestion: please make sure that commit message discusses in the detail how the code works. That hasn't been done.

There're more suggestions: it started with __setattr__, etc. then suddenly there's __getattribute__. Why is it needed, what problem it solves? Please separate it to another pull request, to be thoroughly discussed. Actually, I'd suggest to do the same for each separate piece of the functionality in this mixed-up patch. For example, MicroPython has support for __getattr__, there're tests for it. Is everything ok with it? If yes, title of this pull request if wrong by mentioning __getattr__. If not, what's not as expected, why it's needed, why it can't be done another way, how to implement for MicroPython well? Once that passed review and merged, can be repeated for __setattr__. Then __delattr__. Then __getattribute__.

dmazzella · 2016-12-29T11:53:43Z

@dpgeorge i have used this script for run tests:

try:
    import utime as time
except ImportError:
    import time

class A:
    def __init__(self, d):
        A.d = d

    def __getattr__(self, attr):
        if attr in A.d:
            return A.d[attr]
        else:
            raise AttributeError(attr)

    def __setattr__(self, attr, value):
        A.d[attr] = value

    def __delattr__(self, attr):
        del A.d[attr]

    def test_set(self):
        self.a = self.a + 1
        self.b = self.b + 1
        self.a = self.a
        self.b = self.b
        self.a = self.a
        self.b = self.b
        self.a = self.a
        self.b = self.b

start = time.time()
a = A({"a":1, "b":2})
print(a.a, a.b)
print(time.time() - start)
start = time.time()
for j in range(100):
    a.test_set()
print(time.time() - start)
try:
    start = time.time()
    print(a.a, a.b)
    del a.a
    print(time.time() - start)
    print(a.a)
except AttributeError:
    print("AttributeError")

The results are:

uPy without patch:

dma@dma-pc MSYS ~/micropython/unix
$ ./micropython.exe ../../micropython_dmazzella/tests/basics/delgetsetattr.py
1 2
2.193450927734375e-05
4.291534423828125e-05
101 102
6.914138793945313e-06
1

uPy with patch:

dma@dma-pc MSYS ~/micropython_dmazzella/unix
$ ./micropython.exe ../tests/basics/delgetsetattr.py
1 2
2.288818359375e-05
0.0008180141448974609
101 102
8.821487426757813e-06
AttributeError

cPy:

dma@dma-pc MSYS ~/micropython_dmazzella/unix
$ python3 -B ../tests/basics/delgetsetattr.py
1 2
3.743171691894531e-05
0.0007174015045166016
101 102
9.298324584960938e-06
AttributeError

dmazzella · 2016-12-29T13:42:05Z

@pfalcon ok, if you prefer i can close this pr and split it in more pr.

pfalcon · 2016-12-29T14:14:46Z

That's my personal suggestion, you may want to wait for @dpgeorge's further comments.

dmazzella · 2016-12-29T17:17:13Z

@dpgeorge i have used for example in objobject.c:

mp_obj_type_t *type = mp_obj_get_type(self_in); 
mp_map_t *locals_map = &type->locals_dict->map; 
mp_map_lookup(locals_map, ...

as map where i lookup for attributes, is correct to use this or is more appropriate to use:

mp_obj_instance_t *self = MP_OBJ_TO_PTR(self_in);
mp_map_lookup(&self->members, ...

?

dpgeorge · 2016-12-30T03:00:05Z

It looks like __getattribute__ is incorrectly implemented. It should be called unconditionally before any other access, see https://docs.python.org/3/reference/datamodel.html#object.__getattribute__ . Good tests would find this inconsistency.

Having __getattribute__ would incur a very big performance hit, so let's not add it now. Having __getattr__ should be enough for most cases.

The original discussion started with a need for __setattr__ and __delattr__ so please just implement those to begin with, with full test coverage.

stinos · 2017-01-02T09:47:46Z

@dpgeorge Tested latest version again with this and it's still working (for the rather limited code tested with):

class JsObject(dict):
  def __getattr__(self, name):
    return self[name]

  def __setattr__(self, name, val):
    if isinstance(val, dict):
      val = self.__class__(val)
    self[name] = val

  def __delattr__(self, name):
    return self.__delitem__(name)

o = JsObject(a=1, b=2)
print(o)
print(o.a)
o.b = 3
print(o.b)
o.c = 2
print(o.c)
print(o)
o.d = {'x': 3, 'y' : 5}
print(o.d)
print(o.d.x)
del o.d
print(o)

dmazzella · 2017-01-03T08:07:59Z

ok, I close this, divide into 2 new PR as soon as possible

In relatively unusual circumstances, such as entering `l = 17 ** 17777` at the REPL, you could hit ctrl-c, but not get KeyboardInterrupt. This can lead to a condition where the display would stop updating (micropython#2689).

In micropython#2689, hitting ctrl-c during the printing of an object with a lot of sub-objects could cause the screen to stop updating (without showing a KeyboardInterrupt). This makes the printing of such objects acutally interruptable, and also correctly handles the KeyboardInterrupt: ``` >>> l = ["a" * 100] * 200 >>> l ['aaaaaaaaaaaaaaaaaaaaaa...aaaaaaaaaaa', Traceback (most recent call last): File "<stdin>", line 1, in <module> KeyboardInterrupt: >>> ```

py: Support object __getattr__, __delattr__ and __setattr__

0db9a7f

pfalcon reviewed Dec 15, 2016

View reviewed changes

tests/basics/delgetsetattr.py

del a.a

print(a.a)

except AttributeError:

print("AttributeError")

Copy link

Contributor

pfalcon Dec 15, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing newline.

py: Support object __getattr__, __getattribute__, __delattr__ and __s…

bb688b2

…etattr__ implemented __getattribute__

dmazzella closed this Jan 3, 2017

dmazzella mentioned this pull request Jan 3, 2017

py: object base class doesn't implement __setattr__ and __delattr__ #2755

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

py: Support object getattr, delattr and setattr #2689

py: Support object getattr, delattr and setattr #2689

dmazzella commented Dec 15, 2016 •

edited

pfalcon Dec 15, 2016

pfalcon Dec 15, 2016

pfalcon commented Dec 15, 2016

pfalcon commented Dec 15, 2016

dhylands commented Dec 15, 2016

pfalcon Dec 15, 2016

pfalcon commented Dec 15, 2016

dmazzella commented Dec 15, 2016

dmazzella commented Dec 15, 2016 •

edited

dhylands commented Dec 15, 2016 •

edited

pfalcon commented Dec 15, 2016

pfalcon commented Dec 15, 2016

dmazzella commented Dec 16, 2016 •

edited

dmazzella commented Dec 16, 2016 •

edited

dmazzella commented Dec 16, 2016

dmazzella commented Dec 21, 2016

dmazzella commented Dec 21, 2016

dmazzella commented Dec 21, 2016

dmazzella commented Dec 28, 2016

dpgeorge commented Dec 29, 2016

dpgeorge commented Dec 29, 2016

stinos commented Dec 29, 2016

pfalcon commented Dec 29, 2016

dmazzella commented Dec 29, 2016

dmazzella commented Dec 29, 2016

pfalcon commented Dec 29, 2016

dmazzella commented Dec 29, 2016

dpgeorge commented Dec 30, 2016

stinos commented Jan 2, 2017

dmazzella commented Jan 3, 2017

		@@ -533,6 +533,15 @@ STATIC void mp_obj_instance_load_attr(mp_obj_t self_in, qstr attr, mp_obj_t *des

		// try __getattr__
		if (attr != MP_QSTR___getattr__) {

py: Support object __getattr__, __delattr__ and __setattr__ #2689

py: Support object __getattr__, __delattr__ and __setattr__ #2689

Conversation

dmazzella commented Dec 15, 2016 • edited

pfalcon Dec 15, 2016

Choose a reason for hiding this comment

pfalcon Dec 15, 2016

Choose a reason for hiding this comment

pfalcon commented Dec 15, 2016

pfalcon commented Dec 15, 2016

dhylands commented Dec 15, 2016

pfalcon Dec 15, 2016

Choose a reason for hiding this comment

pfalcon commented Dec 15, 2016

dmazzella commented Dec 15, 2016

dmazzella commented Dec 15, 2016 • edited

dhylands commented Dec 15, 2016 • edited

pfalcon commented Dec 15, 2016

pfalcon commented Dec 15, 2016

dmazzella commented Dec 16, 2016 • edited

dmazzella commented Dec 16, 2016 • edited

dmazzella commented Dec 16, 2016

dmazzella commented Dec 21, 2016

dmazzella commented Dec 21, 2016

dmazzella commented Dec 21, 2016

dmazzella commented Dec 28, 2016

dpgeorge commented Dec 29, 2016

dpgeorge commented Dec 29, 2016

stinos commented Dec 29, 2016

pfalcon commented Dec 29, 2016

dmazzella commented Dec 29, 2016

dmazzella commented Dec 29, 2016

pfalcon commented Dec 29, 2016

dmazzella commented Dec 29, 2016

dpgeorge commented Dec 30, 2016

stinos commented Jan 2, 2017

dmazzella commented Jan 3, 2017

py: Support object getattr, delattr and setattr #2689

py: Support object getattr, delattr and setattr #2689

dmazzella commented Dec 15, 2016 •

edited

dmazzella commented Dec 15, 2016 •

edited

dhylands commented Dec 15, 2016 •

edited

dmazzella commented Dec 16, 2016 •

edited

dmazzella commented Dec 16, 2016 •

edited