Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

add arguments to listener call#4

Closed
jovanepires wants to merge 2 commits into
python-whistle:master from
jovanepires:feature-add-params
Closed

add arguments to listener call #4
jovanepires wants to merge 2 commits into
python-whistle:master from
jovanepires:feature-add-params

Conversation

@jovanepires

@jovanepires jovanepires commented Apr 12, 2020

Copy link
Copy Markdown

Just an improvement I needed and I think it would be interesting for other people.

Copy link
Copy Markdown

Pull Request Test Coverage Report for Build 36

  • 4 of 4 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.2%) to 94.118%

Totals Coverage Status
Change from base Build 31: 0.2%
Covered Lines: 62
Relevant Lines: 64

💛 - Coveralls

coveralls commented Apr 12, 2020
edited
Loading

Copy link
Copy Markdown

Pull Request Test Coverage Report for Build 39

  • 4 of 4 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+1.1%) to 95.05%

Totals Coverage Status
Change from base Build 31: 1.1%
Covered Lines: 61
Relevant Lines: 62

💛 - Coveralls

update requirements
fix coverage

Copy link
Copy Markdown
Author

@hartym could you review this feature?

neronmoon reacted with thumbs up emoji

neronmoon commented Apr 17, 2020
edited
Loading

Copy link
Copy Markdown

Please, review! @hartym

Btw, @jovanepires is this feature can pass arguments as kwargs instead of args? Would be nice!

Copy link
Copy Markdown
Author

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!

hartym commented Dec 9, 2023
edited
Loading

Copy link
Copy Markdown
Member

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 ?

@hartym hartym closed this Dec 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /