8000 MEP22: Navigation by events by fariza · Pull Request #3652 · matplotlib/matplotlib · GitHub
[go: up one dir, main page]

Skip to content

MEP22: Navigation by events #3652

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

Merged
merged 69 commits into from
Apr 9, 2015
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
69 commits
Select commit Hold shift + click to select a range
8cceed4
navigation and toolbar coexistence
fariza Jan 23, 2014
3118a5a
mod keypress in figuremanager
fariza Jan 23, 2014
b4d5fcf
helper methods in toolbar and navigation
fariza Jan 24, 2014
1e8af47
Adding doc to base methods
fariza Jan 24, 2014
622cb95
property for active_toggle
fariza Jan 26, 2014
d1a9de4
simulate click
fariza Jan 27, 2014
3f89d52
activate renamed to trigger
fariza Jan 28, 2014
4f3c10b
toggle tools using enable/disable from its trigger method
fariza Jan 29, 2014
6065daa
simplifying _handle_toggle
fariza Jan 29, 2014
f6a2f19
reducing number of locks
fariza Jan 29, 2014
05db3b6
changing toggle and persistent attributes for issubclass
fariza Feb 4, 2014
c08fe56
bug in combined key press
fariza Feb 4, 2014
b207a72
untoggle zoom and pan from keypress while toggled
fariza Feb 4, 2014
9266447
classmethods for default tools modification
fariza Feb 6, 2014
a53419a
adding zaxis and some pep8
fariza May 1, 2014
704c717
removing legacy method dynamic update
fariza May 6, 2014
5056729
tk backend
fariza May 6, 2014
e6a4e1e
example working with Tk
fariza May 6, 2014
8942c47
duplicate code in keymap tool initialization
fariza Jul 24, 2014
022de6f
grammar corrections
fariza Jul 24, 2014
2c9a195
moving views and positions to tools
fariza Jul 24, 2014
cafe668
The views positions mixin automatically adds the clear as axobserver
fariza Jul 25, 2014
224f745
bug when navigation was not defined
fariza Jul 25, 2014
94c711e
Small refactor so that we first initiate the Navigation (ToolManager)…
OceanWolf Jul 28, 2014
67257e7
Moved default_tool initilisation to FigureManagerBase and cleaned.
OceanWolf Jul 29, 2014
ffa65d6
Temporary fix to backends
OceanWolf Jul 29, 2014
6739ee0
removing persistent tools
fariza Sep 3, 2014
d18206f
removing unregister
fariza Sep 4, 2014
34a52c8
change cursor inmediately after toggle
fariza Sep 5, 2014
c2da483
removing intoolbar
fariza Oct 15, 2014
44a9b0e
events working
fariza Oct 16, 2014
a2ed47f
using pydispatch
fariza Oct 17, 2014
0665890
using navigation as signal handler
fariza Oct 20, 2014
411e6e2
removing view positions singleton
fariza Oct 20, 2014
d484ebd
moving set_cursor completely out of navigation
fariza Oct 27, 2014
75bf97b
removing unused event class
fariza Nov 10, 2014
6cc040b
underscore in tool_trigger-xxx
fariza Nov 10, 2014
0ff5997
adding radio_group for toggle tools
fariza Nov 14, 2014
af6734f
scroll to zoom in zoom/pan tools
fariza Nov 28, 2014
78513d2
remove ToolAddedEvent incorporating the functionality into toolevent
fariza Dec 5, 2014
377ff54
eliminating repeated loop
fariza Jan 5, 2015
7dbbf58
replace draw by draw_idle in tools
fariza Jan 21, 2015
dd66b57
rename mpl_connect
fariza Jan 21, 2015
67a414f
cleaning navigation and toolbar dependencies
fariza Feb 4, 2015
e415d8d
Made NavigationBase.get_tool() more useful.
OceanWolf Feb 11, 2015
1213086
Refactored Toolbar out of NavigationBase
OceanWolf Feb 12, 2015
ba61dec
Some short cuts for adding tools
OceanWolf Feb 16, 2015
9f2ee2b
Lots of fixes
OceanWolf Feb 18, 2015
9da2b13
Rename ToolbarBase -> ToolContainerBase
OceanWolf Feb 18, 2015
110253f
Statusbar
OceanWolf Feb 20, 2015
e2804ea
tool group position
fariza Feb 26, 2015
9a64b7e
docstrings and small corrections by WeatherGod
fariza Mar 23, 2015
64f947f
tkbackend updated
fariza Mar 31, 2015
e8cd5d5
tacaswell comments aprl 1
fariza Apr 1, 2015
4bbcf4e
renaming tool_trigger_event
fariza Apr 1, 2015
73a2661
add_tools moved out of base classes
fariza Apr 1, 2015
1b83628
figure.setter in tools
fariza Apr 1, 2015
e4edd23
rename tools to default_tools to avoid confusion
fariza Apr 1, 2015
d4ac2fb
docstring helper add_tools methods
fariza Apr 1, 2015
a7640ef
rename navigation to toolmanager
fariza Apr 2, 2015
48a6971
tkagg updated for toolmanager and tool groups
fariza Apr 2, 2015
8dafe09
doc and minor code organization
fariza Apr 2, 2015
a0695d0
whats new
fariza Apr 3, 2015
328b169
missing object from class declaration
fariza Apr 3, 2015
aac4744
remove comments and docstrings
fariza Apr 3, 2015
f09b9ef
import with original name backend_tools
fariza Apr 3, 2015
def3a52
rename 2 -> to, example without gtk only code
fariza Apr 7, 2015
9ee7e25
zoom pan buttons order
fariza Apr 7, 2015
5eae4e1
matplotlib.rcParams['toolbar'] == 'None' starts toolmanager but not t…
fariza Apr 7, 2015
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Rename ToolbarBase -> ToolContainerBase
  • Loading branch information
OceanWolf authored and fariza committed Apr 7, 2015
commit 9da2b1324d2304aba39d98a95c0dfe4376c7bc63
101 changes: 47 additions & 54 deletions lib/matplotlib/backend_bases.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
user interaction (key press, toolbar clicks, ..) and the actions in
response to the user inputs.

:class:`ToolbarBase`
:class:`ToolContainerBase`
The base class for the Toolbar class of each interactive backend.

"""
Expand Down Expand Up @@ -3399,18 +3399,16 @@ def remove_tool(self, name):
del self._tools[name]

def add_tools(self, tools):
Copy link
Member

Choose a reason for hiding this comment

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

How strongly do you feel about keeping both the singular and plural versions of this? The similarity (leading to many typos) and the difference in signature/what they can do is going to be confusing. If this function stays I think it needs to be able to pass args/kwargs through to the constructors.

Is there any terrifying input parsing we could use to mush this into one function? I am not seeing a way quickly...

Copy link
Member

Choose a reason for hiding this comment

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

@tacaswell, me and @fariza had a discussion on args/kwargs etcetrra over in fariza#11 where I introduced them.

I would have thought the difference in signature would solve the typos problem, I mean if you make a typo, you will get a python error telling you you passed the wrong number of arguments to the function.

Copy link
Member Author

Choose a reason for hiding this comment

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

We can think of renaming the methods (suggestions any one?), but playing magic with arguments is 👎

Copy link
Member

Choose a reason for hiding this comment

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

great, sold on 👎 for args magic (we have it else where where I do not like it, but supposedly users like that sort of thing)

What is the use case for bulk adding tools? Writing this loop in user code does not strike me as super onerous.

My worry with the typo is getting an exception about args being wrong (which leads you to check what you are passing in) rather than an AttributeError (which leads you to check your spelling).

Copy link
Member Author

Choose a reason for hiding this comment

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

It is more for backend implementation. It was done automatically at __init__ but @OceanWolf talked me out of that

Copy link
Member

Choose a reason for hiding this comment

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

Ooh, brain wave, how about:

for tool in six.iteritems(tools):
    self.add_tool(*tool)

Or get rid of it altogether, I think it has changed a lot (becoming a lot simpler), especially since we split the adding of tools to the toolbar out and into ToolContainer.addTool as @fariza mentions.

Copy link
Member Author

Choose a reason for hiding this comment

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

@OceanWolf where do you suggest to add that loop?
Don't you like the helper functions in backend_tools?

Copy link
Member

Choose a reason for hiding this comment

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

👍 On putting this helper function as a module-level function. Breaks the OO abstraction a bit, but it keeps the base objects simpler.

@OceanWolf That would unpack as self.add_tool(tool_name, value_in_tools_dict) which does not get you the next level of unpacking you need to pass to add_tool.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I like that too, brain just sluggish and getting distracted by quite a few conversations. I don't mind, just so long as no core-logic changes.

Copy link
Member Author

Choose a reason for hiding this comment

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

done

""" Add multiple tools to `Navigation`
""" Add multiple tools to `NavigationBase`

Parameters
----------
tools : List
List in the form
[(Tool1, name1), (Tool2, name2) ...]
where Tool1, name1 represent the tool, and the respective name
of the tool which gets used as an id.
tools : {str: class_like}
The tools to add in a {name: tool} dict, see `add_tool` for more
info.
"""

for tool, name in tools:
for name, tool in six.iteritems(tools):
self.add_tool(name, tool)

def add_tool(self, name, tool, *args, **kwargs):
Expand All @@ -3424,14 +3422,18 @@ def add_tool(self, name, tool, *args, **kwargs):

Parameters
----------
name : string
name : str
Name of the tool, treated as the ID, has to be unique
tool : string or `matplotlib.backend_tools.ToolBase` derived class
Reference to find the class of the Tool to be added
tool : class_like, i.e. str or type
Reference to find the class of the Tool to added.

Notes
-----
args and kwargs get passed directly to the tools constructor.

See Also
--------
matplotlib.backend_tools.ToolBase : The base class for tools.
Copy link
Member

Choose a reason for hiding this comment

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

don't we need backticks around class names to make them links? Also, it used to be that we needed to prepend with :class:. Is that no longer the case?

Copy link
Member Author

Choose a reason for hiding this comment

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

As far as I can see, in the See Also section, the items are automatically converted to links without the backticks. If I add them I get an error while building the docs

"""

tool_cls = self._get_cls_to_instantiate(tool)
Expand All @@ -3442,7 +3444,7 @@ def add_tool(self, name, tool, *args, **kwargs):
if name in self._tools:
Copy link
Member

Choose a reason for hiding this comment

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

Should this be done first in this function? Do we want to check to make sure that they are the same type? Again, why warn rather than raise?

Copy link
Member Author

Choose a reason for hiding this comment

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

I used warn because i don't think that adding twice the same tool represents a major fault and your program will keep working as expected (you already have the tool)

warnings.warn('A tool_cls with the same name already exist, '
Copy link
Member

Choose a reason for hiding this comment

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

a user won't know what tool_cls is because that name is only relevant inside this function. Say "Tool class".

Also, "exist" --> "exists".

'not added')
return
return self._tools[name]

self._tools[name] = tool_cls(self, name, *args, **kwargs)

Expand Down Expand Up @@ -3573,30 +3575,32 @@ def tools(self):

return self._tools

def get_tool(self, name):
def get_tool(self, name, warn=True):
"""Return the tool object, also accepts the actual tool for convenience

Parameters
-----------
name : String, ToolBase
name : str, ToolBase
Name of the tool, or the tool itself
warn : bool
Copy link
Member

Choose a reason for hiding this comment

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

, optional

Copy link
Member Author

Choose a reason for hiding this comment

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

done

If this method should give warnings.
"""
if isinstance(name, tools.ToolBase) and tool.name in self._tools:
if isinstance(name, tools.ToolBase) and name.name in self._tools:
return name
if name not in self._tools:
warnings.warn("%s is not a tool controlled by Navigation" % name)
if warn:
warnings.warn("Navigation does not control tool %s" % name)
return None
return self._tools[name]


class ToolbarBase(object):
"""Base class for `Toolbar` implementation
class ToolContainerBase(object):
"""Base class for all tool containers, e.g. toolbars.

Attributes
Copy link
Member

Choose a reason for hiding this comment

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

This isn't lined up with the underscores below

Copy link
Member Author

Choose a reason for hiding this comment

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

done

----------
manager : `FigureManager` object that integrates this `Toolbar`
navigation : `NavigationBase` object that hold the tools that
this `Toolbar` wants to communicate with
navigation : `NavigationBase` object that holds the tools that
this `ToolContainer` wants to communicate with.
"""

def __init__(self, navigation):
1D70 Expand All @@ -3618,51 +3622,38 @@ def _tool_toggled_cbk(self, event):
self.toggle_toolitem(event.tool.name, event.tool.toggled)

def add_tools(self, tools):
""" Add multiple tools to `Navigation`
""" Add multiple tools to the container.

Parameters
----------
tools : List
tools : list
List in the EE58 form
[[group1, [name1, name2 ...]][group2...]]
where group1 is the name of the group where the
Tool1, Tool2... are going to be added, and name1, name2... are the
names of the tools
[[group1, [tool1, tool2 ...]], [group2, [...]]]
Where the tools given by tool1, and tool2 will display in group1.
See `add_tool` for details.
"""

for group, grouptools in tools:
for position, tool in enumerate(grouptools):
self.add_tool(tool, group, position)

def add_tool(self, tool, group, position=-1, name=None, **kwargs):
"""Adds a tool to the toolbar
def add_tool(self, tool, group, position=-1):
"""Adds a tool to this container

Parameters
----------
tool : string, tool
The name or the type of tool to add.
group : string
tool : tool_like
The tool to add, see `NavigationBase.get_tool`.
group : str
The name of the group to add this tool to.
position : int
the relative position within the group to place this tool.
name : string (optional)
If given, and the above fails, we use this to create a new tool of
type given by tool, and use this as the name of the tool.
"""
t = self.navigation.get_tool(tool)
if t is None:
if isinstance(tool, type):
tool = tool.__class__
if name is not None:
t = self.navigation.add_tool(name, tool, **kwargs)
if t is None:
warning.warn('Cannot add tool %s'%tool)
return
tool = t
position : int (optional)
The position within the group to place this tool. Defaults to end.
"""
tool = self.navigation.get_tool(tool)
image = self._get_image_filename(tool.image)
toggle = getattr(tool, 'toggled', None) is not None
self.add_toolitem(tool.name, group, position, image,
tool.description, toggle)
self.add_toolitem(tool.name, group, position,
image, tool.description, toggle)
if toggle:
self.navigation.nav_connect('tool_trigger_%s' % tool.name,
self._tool_toggled_cbk)
Expand All @@ -3688,13 +3679,13 @@ def trigger_tool(self, name):
Parameters
----------
name : String
Name(id) of the tool triggered from within the toolbar
Name(id) of the tool triggered from within the container

"""
self.navigation.tool_trigger_event(name, sender=self)

def add_toolitem(self, name, group, position, image, description, toggle):
"""Add a toolitem to the toolbar
"""Add a toolitem to the container

This method must get implemented per backend

Expand Down Expand Up @@ -3734,18 +3725,20 @@ def set_message(self, s):

pass

def toggle_toolitem(self, name):
def toggle_toolitem(self, name, toggled):
"""Toggle the toolitem without firing event

Parameters
----------
name : String
Id of the tool to toggle
toggled : bool
Whether to set this tool as toggled or not.
"""
raise NotImplementedError

def remove_toolitem(self, name):
"""Remove a toolitem from the `Toolbar`
"""Remove a toolitem from the `ToolContainer`

This method must get implemented per backend

Expand Down
34 changes: 18 additions & 16 deletions lib/matplotlib/backend_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -903,23 +903,25 @@ def _mouse_move(self, event):
self.navigation.canvas.draw_idle()


tools = [(ToolHome, 'home'), (ToolBack, 'back'), (ToolForward, 'forward'),
(ToolZoom, 'zoom'), (ToolPan, 'pan'),
('ToolConfigureSubplots', 'subplots'),
('ToolSaveFigure', 'save'),
(ToolGrid, 'grid'),
(ToolFullScreen, 'fullscreen'),
(ToolQuit, 'quit'),
(ToolEnableAllNavigation, 'allnav'),
(ToolEnableNavigation, 'nav'),
(ToolXScale, 'xscale'),
(ToolYScale, 'yscale'),
(ToolCursorPosition, 'position'),
(ToolViewsPositions, 'viewpos'),
('ToolSetCursor', 'cursor'),
('ToolRubberband', 'rubberband')]
tools = {'home': ToolHome, 'back': ToolBack, 'forward': ToolForward,
'zoom': ToolZoom, 'pan': ToolPan,
'subplots': 'ToolConfigureSubplots',
'save': 'ToolSaveFigure',
'grid': ToolGrid,
'fullscreen': ToolFullScreen,
'quit': ToolQuit,
'allnav': ToolEnableAllNavigation,
'nav': ToolEnableNavigation,
'xscale': ToolXScale,
'yscale': ToolYScale,
'position': ToolCursorPosition,
'viewpos': ToolViewsPositions,
'cursor': 'ToolSetCursor',
Copy link
Member

Choose a reason for hiding this comment

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

why are some of these values strings and some of them classes?

Copy link
Member Author

Choose a reason for hiding this comment

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

The string values are going to be implemented by the backend and NavigationBase._get_cls_to_instantiate will get the right class to instantiate

'rubberband': 'ToolRubberband'}
"""Default tools"""

toolbar_tools = [['navigation', ['home', 'back', 'forward']],
['zoompan', ['zoom', 'pan']],
['layout', ['subplots']],
['io', ['save']]]
"""Default tools"""
"""Default tools in the toolbar"""
6 changes: 3 additions & 3 deletions lib/matplotlib/backends/backend_gtk3.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ def fn_name(): return sys._getframe(1).f_code.co_name
from matplotlib._pylab_helpers import Gcf
from matplotlib.backend_bases import RendererBase, GraphicsContextBase, \
FigureManagerBase, FigureCanvasBase, NavigationToolbar2, cursors, TimerBase
from matplotlib.backend_bases import ShowBase, ToolbarBase, NavigationBase
from matplotlib.backend_bases import ShowBase, ToolContainerBase, NavigationBase
from matplotlib.backend_tools import SaveFigureBase, ConfigureSubplotsBase, \
tools, toolbar_tools, SetCursorBase, RubberbandBase

Expand Down Expand Up @@ -756,9 +756,9 @@ def draw_rubberband(self, x0, y0, x1, y1):
ToolRubberband = RubberbandGTK3


class ToolbarGTK3(ToolbarBase, Gtk.Box):
class ToolbarGTK3(ToolContainerBase, Gtk.Box):
def __init__(self, navigation):
ToolbarBase.__init__(self, navigation)
ToolContainerBase.__init__(self, navigation)
Gtk.Box.__init__(self)
self.set_property("orientation", Gtk.Orientation.VERTICAL)

Expand Down
0