adafruit / circuitpython

CircuitPython - a Python implementation for teaching coding with microcontrollers
https://circuitpython.org
Other
4.02k stars 1.19k forks source link

Overriding and then setting a parent property doesn't work on a subclass of Group #4423

Open lesamouraipourpre opened 3 years ago

lesamouraipourpre commented 3 years ago

Following on from a comment by @kmatch98 on adafruit/Adafruit_CircuitPython_Display_Text#116 about the difficulty subclassing/overriding Group, I've been trying to subclass Label with the example class GridLabel below.

# test polymorphism

import board
import displayio
import terminalio
import time

from adafruit_display_text import label

class GridLabel(label.Label):
    def __init__(self, font, **kwargs):
        super().__init__(font, **kwargs)

    @property
    def x(self):
        return super().x

    @x.setter
    def x(self, value):
        # Round down to the nearest 10 grid
        xx = (value // 10) * 10

        # If Python properly implemented polymorphism the following would work:
        #
        # super().x = xx
        #
        # Unfortunately, thanks to a bug unfixed for 9 years
        # Ref: https://bugs.python.org/issue14965
        # we need to do this
        super(self.__class__, self.__class__).x.__set__(self, xx)

    @property
    def y(self):
        return super().y

    @y.setter
    def y(self, value):
        # Round down to the nearest 10 grid
        yy = (value // 10) * 10

        # If Python properly implemented polymorphism the following would work:
        #
        # super().y = yy
        #
        # Unfortunately, thanks to a bug unfixed for 9 years
        # Ref: https://bugs.python.org/issue14965
        # we need to do this
        super(self.__class__, self.__class__).y.__set__(self, yy)

def main():
    global board
    if hasattr(board, 'DISPLAY'):
        display = board.DISPLAY
    else:
        from adafruit_blinka import board
        from blinka_displayio_pygamedisplay import PyGameDisplay
        # The key here is "auto_refresh=False". Failing to do so will create a new thread
        # for the rendering, and macOS (or the Python implementation for it) can only be
        # performed on the main (UI) thread.
        display = PyGameDisplay(width=320, height=240, auto_refresh=False)

    splash = displayio.Group(max_size=10)
    display.show(splash)

    WIDTH = display.width
    HEIGHT = display.height
    print(f"Display: {WIDTH}x{HEIGHT}")

    # Draw a black background
    bg_bitmap = displayio.Bitmap(WIDTH, HEIGHT , 1)
    bg_palette = displayio.Palette(1)
    bg_palette[0] = 0x000000
    splash.append(displayio.TileGrid(bg_bitmap, pixel_shader=bg_palette))

    sample = GridLabel(terminalio.FONT,
                       text="Polymorph",
                       color=0xFFFFFF,
                       scale=2,
                       x=0,
                       y=0)
    splash.append(sample)

    while True:
        for x in range(0, HEIGHT):
            sample.x = x
            sample.y = x
            print(f"Asked for{x}X{x}  Got {sample.x}X{sample.y}")
            if hasattr(display, 'refresh'):
                display.refresh()
            time.sleep(0.02)
        pass

main()

This code works as expected on a Raspberry Pi, but crashes on a PyPortal Pynt with the following:

Adafruit CircuitPython 6.2.0-beta.2 on 2021-02-11; Adafruit PyPortal with samd51j20
>>> %Run -c $EDITOR_CONTENT
Display: 320x240
Traceback (most recent call last):
  File "<stdin>", line 95, in <module>
  File "<stdin>", line 76, in main
  File "<stdin>", line 12, in __init__
  File "/lib/adafruit_display_text/label.py", line 107, in __init__
  File "<stdin>", line 30, in x
ValueError: Must be a Group subclass.

This error is raised from circuitpython/shared-bindings/displayio/Group.c in the method:

// Helper to ensure we have the native super class instead of a subclass.
displayio_group_t* native_group(mp_obj_t group_obj) {
    mp_obj_t native_group = mp_instance_cast_to_native_base(group_obj, &displayio_group_type);
    if (native_group == MP_OBJ_NULL) {
        mp_raise_ValueError_varg(translate("Must be a %q subclass."), MP_QSTR_Group);
    }
    mp_obj_assert_native_inited(native_group);
    return MP_OBJ_TO_PTR(native_group);
}

As my example code uses the super() proxy object to call the parent set method and is failing the mp_instance_cast_to_native_base() method I assume this method is not able to handle a proxy object and returns MP_OBJ_NULL.

As far as I can tell my example is not unreasonable and should work in CircuitPython.

makermelissa commented 3 years ago

Transferring to CircuitPython as this sounds like a change is required in CircuitPython and not on the Pi.

dhalbert commented 10 months ago

@tannewt might this have been fixed?

tannewt commented 9 months ago

This is not fixed. I tested it at f3920c49dc7c717beb630ada61e61d75de2cf60d.

It looks like the native_group() call is being a passed a super object and mp_obj_cast_to_native_base() doesn't know how to convert that to the native base. I'm not sure if that's what it should do or if it shouldn't be passed the super object in the first place.

A non-displayio example would be really helpful because then we could add a test for it and debug via the unix port.

jepler commented 9 months ago

I wrote a gist that shows what may be as many as 3 distinct problems with subclassing, properties, & super(). The test uses NativeBaseClass, a class that @tannewt recently added to the unix coverage build for exactly this sort of testing (thanks!). This test runs on only on the "unix coverage build", not on microcontrollers.

class N(NativeBaseClass):

    @property
    def subtest(self):
        return super().test

    @subtest.setter
    def subtest(self, value):
        # BUG: In this context something prevents the compiler from
        # transforming the super() call to the 2-arg version
        # which results in
        # "TypeError: function takes 2 positional arguments but 0 were given"
        super().test = value

        # This alternative gets further along (allows triggering AttributeError
        # incorrectly instead):
        super(N, self).test = value

n = N()
n.test = 7
print(n.test)
print(n.subtest) # BUG: segfaults on this line

# BUG: AttributeError: can't set attribute 'test'
n.subtest = 8
print(n.test)
print(n.subtest)