kxgames / glooey

An object-oriented GUI library for pyglet.
MIT License
91 stars 6 forks source link

Incompatible With the Latest Autoprop Release #51

Closed marky1991b closed 3 years ago

marky1991b commented 3 years ago

Given this test script:

import glooey

class TilePickerButton(glooey.Widget):
    custom_alignment = 'center'
    custom_propagate_mouse_events = True

    def __init__(self, cls, on_mouse_press):
        super().__init__()
        self.cls = cls

        vbox = glooey.VBox()
        self.image = glooey.Image(cls.image)
        self.label = glooey.Label(cls.__name__)

        vbox.pack(self.image)
        vbox.add(self.label)
        import pdb
        #pdb.set_trace()
        vbox.padding = 6

        self._attach_child(vbox)
        self.on_mouse_press_func = on_mouse_press

    def on_mouse_press(self, x, y, button, modifiers):
        super().on_mouse_press(x, y, button, modifiers)
        print("pressed")
        return self.on_mouse_press_func()

class FakeSprite:
    image = "fake.png"

cls = FakeSprite
button = TilePickerButton(cls, lambda cls=cls: setattr(self, "active_content_cls", cls))

This code works using autoprop 0.0.3, but fails with the latest version, 3.0.0. The error:

  File "test_glooey.py", line 33, in <module>
    button = TilePickerButton(cls, lambda cls=cls: setattr(self, "active_content_cls", cls))
  File "test_glooey.py", line 19, in __init__
    vbox.padding = 6
AttributeError: can't set attribute

This is happening because autoprop (at least as of 3.0.0) specifically validates that the setter accepts exactly one argument other than self and must be passed positionally. In older versions, it wasn't so strict (or at least, this example works when i run it using autoprop version 0.0.3) , but I checked and the documentation has always at least claimed this to be a requirement.

kalekundert commented 3 years ago

Thanks for the bug report; I'll have to look into this ASAP. I breifly looked at HVBox.set_padding() method, and it seems to accept exactly one positional argument and several optional keyword arguments. autoprop should be ok with that, so this might actually be a bug in in autoprop.

marky1991b commented 3 years ago

Looking at my copy of HVBox, this is the definition: def set_padding(self, all=None, *, horz=None, vert=None, left=None, right=None, top=None, bottom=None, cell=None):

In this case, there's no required positional argument (all defaults to None), which is what's causing autoprop to reject it.

Here's my research, in case it helps:

Autoprop's old code:

            num_args = len(arg_spec.args) - len(arg_spec.defaults or ())
            num_args_minus_self = num_args - 1

            if num_args_minus_self != expected_num_args[prefix]:
                return False

In a shell:

(Pdb) arg_spec = inspect.getfullargspec(attr)
(Pdb) num_args = len(arg_spec.args) - len(arg_spec.defaults or ())
(Pdb) num_args
2
# it passes validations

Autoprop's new code:

    prefix, name = accessor_match.groups()
    sig = inspect.signature(attr)
    params = sig.parameters.values()
    num_args = sum(map(is_required, params)) - 1
(Pdb) params
_OrderedDictValuesView([<Parameter "self">, <Parameter "all=None">, <Parameter "horz=None">, <Parameter "vert=None">, <Parameter "left=None">, <Parameter "right=None">, <Parameter "top=None">, <Parameter "bottom=None">])
[_ for _ in map(is_required, params)]     
[True, False, False, False, False, False, False, False]
(Pdb) num_args
0
(Pdb) _EXPECTED_NUM_ARGS[prefix]
1
(Pdb) num_args != _EXPECTED_NUM_ARGS[prefix]
True
# hence it rejects set_padding
kalekundert commented 3 years ago

Ah, I see. Thanks for doing this research. I'll have to fix the logic in autoprop, but that shouldn't be too hard.

marky1991b commented 3 years ago

Is there any update on this? I would make the fix myself and submit a PR, but I'm not sure what the desired fix is.

kalekundert commented 3 years ago

Sorry for being slow. I took this as an opportunity to make a couple big changes I had in mind for autoprop, but I kinda stalled. I'll push what I have later today; it should be enough to fix this bug.

kalekundert commented 3 years ago

Ok, I just released autoprop 4.0, which should fix this bug. Let me know if you still have any problems.

marky1991b commented 3 years ago

I can confirm this fixed the issue. Thank you!