theodox / mGui

Python module for cleaner maya GUI layout syntax
MIT License
123 stars 23 forks source link

MenuItemProxy items fail to register internal callables #45

Closed Mouthlessbobcat closed 7 years ago

Mouthlessbobcat commented 7 years ago

CallbackProxy instances created via the get_weak_reference factory method don't have the correct name attribute of their internal function object.

Traceback (most recent call last):
  File "C:\ULDev\source\ulMaya\external\mGui\mGui\menu_loader.py", line 165, in instantiate
    new_item.command += cp
  File "C:\ULDev\source\ulMaya\external\mGui\mGui\events.py", line 87, in _add_handler
    self._Handlers.add(get_weak_reference(handler))
  File "C:\ULDev\source\ulMaya\external\mGui\mGui\events.py", line 237, in get_weak_reference
    return WeakMethodFree(f)
  File "C:\ULDev\source\ulMaya\external\mGui\mGui\events.py", line 213, in __init__
    self._ref_name = f.__name__
AttributeError: 'CallbackProxy' object has no attribute '__name__'
Mouthlessbobcat commented 7 years ago

Proposed fix is to override CallbackProxy's _ name _ to its wrapped internal function.

    def __name__(self):
        return self.func.__name__

Would it be useful/correct to implement the adapter pattern so that all missing attributes on CallbackProxy are redirected to its wrapped func?

    def __getattr__(self, attr):
        if hasattr(self.func, attr):
            return getattr(self.func, attr)
        else:
            raise AttributeError("{} object has no attribute {}".format(type(self), attr))
theodox commented 7 years ago

@bob-white I think this originated in one of yours - what was the original intent? Is there a matching change in menuLoader that makes this not an issue?

bob-white commented 7 years ago

The original intent was to provide clearer messages during a DeadReferenceError, specifically identifying which function had slipped out of scope. However I forgot that not all callables have a __name__ attribute.

It looks like the issue is that menu_loader is wrapping its callbacks in the CallbackProxy object, specifically so that the functions remain in scope similar to #40.

I can put a patch together that will just default a name of 'unnamed' if the callable lacks the __name__ attribute, and add the attribute to the CallbackProxy class so that it benefits from the better error messages.

theodox commented 7 years ago

@Mouthlessbobcat - does bob's fix sound right to you?

Mouthlessbobcat commented 7 years ago

@bob-white , @theodox That change sounds good to me! Thanks for looking into it.

theodox commented 7 years ago

@Mouthlessbobcat - merged. I'll close the issue when we're sure it works!

theodox commented 7 years ago

@Mouthlessbobcat can I close this guy out?

Mouthlessbobcat commented 7 years ago

@theodox Yup! Looks fine to me.