decorators: add __self__ #70
Conversation
qcore/decorators.py
Outdated
| @@ -48,6 +48,8 @@ class DecoratorBinder(object): | |||
| def __init__(self, decorator, instance=None): | |||
| self.decorator = decorator | |||
| self.instance = instance | |||
| if instance is not None: | |||
| self.__self__ = instance | |||
There was a problem hiding this comment.
Setting __self__ only for bound methods is consistent with how normal Python methods behave, since unbound methods don't have __self__. On the other hand, it's a bit weird for an attribute to only be present on the same type of object some of the time, so maybe we should just set __self__ to None in the unbound case.
What do you think @besfahbod?
There was a problem hiding this comment.
Thanks. Yeah, I agree. Here's my reasoning:
- It does makes sense for PY to do so in the basic lang-item object types, keeping the API surface as small as possible.
- In this library, however, we're dealing with higher-level objects, specially ones that can work on multiple lang-item types. If we did have generics support over those types, we probably could be more specific. But since we don't, it's good to keep the API consistent regardless of the type of lang-item.
So, should we just drop the condition check, then?
There was a problem hiding this comment.
Sounds good, done
|
This caused some problems internally because some code assumes that anything with |
|
@besfahbod do you mind if I revert this to avoid trouble with other libraries? Deploying the change seems risky now and it's easy for tools that want to work with decorators to use |
This reverts commit 2d143c0.
|
Yeah, sure. Let's just revert, and we take another stab at it along with support for |
Fixes quora/asynq#102.