Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Central dispatcher TODELETE #3663

Closed
wants to merge 42 commits into from
Closed

Conversation

fariza
Copy link
Member

@fariza fariza commented Oct 17, 2014

Another variation for MEP22 PR #2759 #3652
This time using pydispatch as central signal event dispatcher, to communicate between toolbar, navigation and tools.
The use of pydispatch is just for simplicity, and can be replaced with custom code. But the idea, is to implement a central signal handler, that everybody can connect and send to.

@fariza
Copy link
Member Author

fariza commented Oct 17, 2014

The central dispatcher pydispatch can be easily replaced with one using cbook.CallbackRegistry()

@jenshnielsen
Copy link
Member

To get the tests passing you need to make sure pydispatch is installed on travis. This should probably go into setup like the rest of the dependencies

@fariza
Copy link
Member Author

fariza commented Oct 17, 2014

Thanks.
For the moment it is only a proof of concept for an "independent signal
manager". If we go this way in the MEP22 I will replace pydispatch with
something similar using the Callback stuff (matplotlib).

I hope for the review there is no need of passing test.

Federico
On 17 Oct 2014 18:20, "Jens Hedegaard Nielsen" notifications@github.com
wrote:

To get the tests passing you need to make sure pydispatch is installed on
travis. This should probably go into setup like the rest of the dependencies


Reply to this email directly or view it on GitHub
#3663 (comment)
.

@jenshnielsen
Copy link
Member

In that case i suggest that you just add pydispatch to .travis.yml (the line that uses pep to install dependencies) That is a simple way of getting the tests running and we can just remove it again when pydispatch is gone.

@tacaswell tacaswell added this to the v1.5.x milestone Oct 18, 2014
@tacaswell
Copy link
Member

From your descriptions (and with out diving into the code) I like this one best.

@fariza
Copy link
Member Author

fariza commented Oct 18, 2014

@tacaswell I agree
The only thing is:
Do we want a central signal/dispatcher?
Canvas is it's own dispatcher, so, if we add this we duplicate work, unless... and this is a big UNLESS in the future we want to take the signal handling out of canvas, and it will just emit using the central dispatcher.

@tacaswell
Copy link
Member

Given the centrality of the canvas to the rest of the event handling system and the way that the canvas classes are used as the hook into the gui backend's events I don't think that is going anywhere.

@fariza
Copy link
Member Author

fariza commented Oct 18, 2014

OK. In that case the question is:
Do we want a central handler for the rest of the world (except canvas), or
just one for the tools?

If it is just for the tools navigation can assume that role.
On 18 Oct 2014 00:36, "Thomas A Caswell" notifications@github.com wrote:

Given the centrality of the canvas to the rest of the event handling
system and the way that the canvas classes are used as the hook into the
gui backend's events I don't think that is going anywhere.


Reply to this email directly or view it on GitHub
#3663 (comment)
.

@tacaswell tacaswell modified the milestones: v1.5.x, unassigned Oct 18, 2014
@WeatherGod
Copy link
Member

I am very intrigued by this as well. My book makes extensive use of the
canvas's callback registry as well as callbacks just about everywhere else.

One of the implicit reasonings behind the decentralized event dispatcher is
the ability to localize events. For example, if I have multiple figure up
and I add a callback to an event for a particular figure, I expect the
callback to only apply to the event in that figure. Also, just about all of
the widgets have their own callback registry's, because it makes sense for
the event to only be emitted for that particular instance of the widget. If
I have two slider widgets, and attached callback1 to the first slider and
callback2 to the second slider, it doesn't mean that I want both callbacks
triggered when the same-named event is emitted from one of the sliders. Of
course, we could take care to uniquify events down to the centralized
dispatcher, but what would we gain?

I am not entirely -1 on this, as the dispatcher is ancient. I need to read
through this proposals more carefully. I like how you are providing
multiple takes on this MEP. It really helps to explore the possibilities.

On Sat, Oct 18, 2014 at 12:40 AM, Federico Ariza notifications@github.com
wrote:

OK. In that case the question is:
Do we want a central handler for the rest of the world (except canvas), or
just one for the tools?

If it is just for the tools navigation can assume that role.
On 18 Oct 2014 00:36, "Thomas A Caswell" notifications@github.com
wrote:

Given the centrality of the canvas to the rest of the event handling
system and the way that the canvas classes are used as the hook into the
gui backend's events I don't think that is going anywhere.


Reply to this email directly or view it on GitHub
<
https://github.com/matplotlib/matplotlib/pull/3663#issuecomment-59598853>
.


Reply to this email directly or view it on GitHub
#3663 (comment)
.

@fariza
Copy link
Member Author

fariza commented Oct 19, 2014

I thought about having a separate callback registry in the tools, but then I have to have one in Navigation and also one in Toolbar.
Doing it that way, it becomes a nightmare too many places to connect to and fire events to/from.

In terms of identifying the source of the event, it can be done several ways, at connect time, we can specify the source/s that I want to listen to. Or at event time filter by the source of the event.

But maybe, the best solution is goint PR #3652 way

  • One callback registry for tool related stuff inside Navigation
  • Toolbar and tools connect to and fire events trough Navigation.
  • Navigation listen to its own events to perform tool syncronization (toogle/untoggle)

@fariza fariza changed the title Central dispatcher Central dispatcher TODELETE Oct 21, 2014
@fariza fariza closed this Oct 23, 2014
@fariza fariza deleted the central-dispatcher branch February 11, 2015 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants