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

MEP22: Navigation by events #3652

Merged
merged 69 commits into from Apr 9, 2015
Merged

Conversation

fariza
Copy link
Member

@fariza fariza commented Oct 16, 2014

This PR is the implementation of MEP22, https://github.com/matplotlib/matplotlib/wiki/Mep22#implementation

This it supersedes the #2759

Toolbar has been relegated as listener/emiter of events that get dispatched to the Tools by Navigation.

The toolbar is easily reconfigurable at running time. new tools can be created/added/removed without modificaion of the backend code.

Example
examples/user_interfaces/navigation.py

@tacaswell tacaswell modified the milestone: v1.5.x Oct 17, 2014
@WeatherGod
Copy link
Member

I am definitely very interested in looking through this this weekend. At a
high-level at least, this is along the lines of what I was thinking of
doing with the keymaps (i.e., map the keys to named events) which would
make for simpler code and configuration, and allows for user-extensibility.

On Thu, Oct 16, 2014 at 5:45 PM, Federico Ariza notifications@github.com
wrote:

This is an slightly different alternative to PR #2759
#2759 for MEP22 where
Navigation implements some events to communicate with the clients (Toolbar,
etc...), this allows to have multiple entry points for interacting with

Tools

You can merge this Pull Request by running

git pull https://github.com/fariza/matplotlib navigation-by-events

Or view, comment on, or merge it at:

#3652
Commit Summary

  • navigation and toolbar coexistence
  • mod keypress in figuremanager
  • extra files
  • helper methods in toolbar and navigation
  • Adding doc to base methods
  • property for active_toggle
  • simulate click
  • pep8 backend_tools
  • activate renamed to trigger
  • toggle tools using enable/disable from its trigger method
  • simplifying _handle_toggle
  • reducing number of locks
  • pep8 correction
  • changing toggle and persistent attributes for issubclass
  • bug in combined key press
  • untoggle zoom and pan from keypress while toggled
  • classmethods for default tools modification
  • six fixes
  • adding zaxis and some pep8
  • removing legacy method dynamic update
  • tk backend
  • pep8
  • example working with Tk
  • cleanup
  • duplicate code in keymap tool initialization
  • grammar corrections
  • moving views and positions to tools
  • The views positions mixin automatically adds the clear as axobserver
  • bug when navigation was not defined
  • Small refactor so that we first initiate the Navigation
    (ToolManager), before filling it with tools.
  • Update for Sphinx documentation
  • Moved default_tool initilisation to FigureManagerBase and cleaned.
  • Fix navigation
  • Temporary fix to backends
  • Merge pull request Fix autofmt_xdate() when using in conjunction with twinx() #1 from
    OceanWolf/navigation-toolbar-coexistence-event-framework
  • removing persistent tools
  • removing unregister
  • change cursor inmediately after toggle
  • removing intoolbar
  • events working

File Changes

Patch Links:


Reply to this email directly or view it on GitHub
#3652.

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

fariza commented Oct 20, 2014

@tacaswell @WeatherGod @OceanWolf
After long traffic thinking time, I am pretty sure this is the way to go.
I applied the latest comments made by @tacaswell on #2759 except for

  • dictionnary vs classes (tool definition)
  • history singleton

If you agree, I will remove the other two PR's and we can keep going.

@tacaswell
Copy link
Member

Sounds good to me. I am still not convinced that going with a zoo of classes is the right thing to do and am still not happy with adding more global state.

@fariza
Copy link
Member Author

fariza commented Oct 20, 2014

I removed the singleton, I added another tool, that keeps the "history". The other tools access this one to push things around in history

@fariza
Copy link
Member Author

fariza commented Oct 21, 2014

@tacaswell please tag this as "need revision" and remove it from #2759

@tacaswell tacaswell modified the milestones: v1.5.x, unassigned Oct 21, 2014
@fariza
Copy link
Member Author

fariza commented Oct 31, 2014

@WeatherGod did you have time to check it out?

@WeatherGod
Copy link
Member

Hopefully, I'll have time this weekend... (book-writing is hard!)

On Fri, Oct 31, 2014 at 9:41 AM, Federico Ariza notifications@github.com
wrote:

@WeatherGod https://github.com/WeatherGod did you have time to check it
out?


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

@fariza fariza changed the title Navigation by events MEP22: Navigation by events Nov 7, 2014
"""Event for navigation tool management (add/remove/message)"""
def __init__(self, name, sender, **kwargs):
ToolEvent.__init__(self, name, sender)
for key, value in kwargs.items():
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Patterns like this make me nervous, might as well just pass around a dict.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed

@fariza
Copy link
Member Author

fariza commented Nov 14, 2014

@tacaswell I gave up (too tempting).
I just added the radio_group attribute for ToolToggleBase derived tools, now it is possible to specify a group that transforms the tools in that group as "mutually exclusive" (untoggle one before toggling the other).
As per your suggestion, ToolGrid, ToolFullScreen, ToolYScale and ToolXScale are now toggle tools.

@OceanWolf
Copy link
Contributor

Okay, merge time?

@WeatherGod
Copy link
Member

Congratulations, @fariza and @OceanWolf ! You are now proud owners of some mpl real estate!

WeatherGod added a commit that referenced this pull request Apr 9, 2015
@WeatherGod WeatherGod merged commit cd5edcf into matplotlib:master Apr 9, 2015
@fariza
Copy link
Member Author

fariza commented Apr 9, 2015

Thank you everybody.

Actually I am going to present a Lightning Talk at Pycon2015
Here is the notebook just in case somebody is interested https://github.com/fariza/pycon2015/blob/master/ToolDemo.ipynb

@OceanWolf
Copy link
Contributor

@fariza nice, though the file looks nonsense on github ;). When do you give the talk? Perhaps I can get MEP27 ready by then ;).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
MEP: MEP22 tool manager
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants