-
Notifications
You must be signed in to change notification settings - Fork 5
Conversation
coveralls
commented
Apr 12, 2020
Pull Request Test Coverage Report for Build 36
💛 - Coveralls |
Pull Request Test Coverage Report for Build 39
💛 - Coveralls |
update requirements fix coverage
9635ecd to
e409bb9
Compare
jovanepires
commented
Apr 16, 2020
@hartym could you review this feature?
Please, review! @hartym
Btw, @jovanepires is this feature can pass arguments as kwargs instead of args? Would be nice!
jovanepires
commented
Apr 19, 2020
Please, review! @hartym
Btw, @jovanepires is this feature can pass arguments as kwargs instead of args? Would be nice!
Not currently, but I could implement it.
Thank you for your support!
Very sorry for not seeing this, I'm very bad with notifications and whistle being very simple and stable, I don't come there often (although I'm currently working on a 2.0 supporting async events).
I don't really see why this would be useful. Usually, I create an Event class that contains all required parameters, and pass it to the dispatch method (see below for example).
Also, current usage encourage writing things like:
event = SomeEvent() dispatcher.dispatch('foo', event)
With your patch, there would be an unwanted side effect of considering event as the first element of args instead of event, which changes the general usage behaviour. We could write code to consider the first element of args as event value if event is None, but that would be a little dubious code.
To achieve your use case, please just create your event class and embed your arguments in it:
class MyEvent(Event): def __init__(self, foo, bar): self.foo = foo self.bar = bar dispatcher.dispatch("hello", MyEvent('foo', 'bar')) def handler(event: MyEvent): print(event.foo, event.bar)
This way you can implement whatever you want, without adding arguments magic into the dispatcher.
Is there a concrete use case that you implement with *args that you can not implement by having your own event class ?
Just an improvement I needed and I think it would be interesting for other people.