Skip to content

removing name argument from tools #9966

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

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 12 additions & 0 deletions doc/api/next_api_changes/behavior/9966-FAR.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
Tool name removal
~~~~~~~~~~~~~~~~~~

The name attribute of the tools `matplotlib.backend_tools` was removed.

When adding a tool to any Tool Container (ex. Toolbars) `matplotlib.backend_bases.ToolContainerBase.add_tool` ,
it is now needed to pass the name of the tool.

When creating new tools, or initializing existing tools, the initialization doesn't accept the tool name anymore `matplotlib.backend_tools.ToolBase`

When creating a tool event `matplotlib.backend_managers.ToolEvent` its necessary to pass the tool name

18 changes: 9 additions & 9 deletions lib/matplotlib/backend_bases.py
Original file line number Diff line number Diff line change
Expand Up @@ -3306,40 +3306,40 @@ def __init__(self, toolmanager):
lambda event: self.set_message(event.message))
toolmanager.toolmanager_connect(
'tool_removed_event',
lambda event: self.remove_toolitem(event.tool.name))
lambda event: self.remove_toolitem(event.tool_name))

def _tool_toggled_cbk(self, event):
"""
Capture the 'tool_trigger_[name]'

This only gets used for toggled tools.
"""
self.toggle_toolitem(event.tool.name, event.tool.toggled)
self.toggle_toolitem(event.tool_name, event.tool.toggled)

def add_tool(self, tool, group, position=-1):
def add_tool(self, tool_name, group, position=-1):
"""
Add a tool to this container.

Parameters
----------
tool : tool_like
The tool to add, see `.ToolManager.get_tool`.
tool_name : str
Copy link
Member

Choose a reason for hiding this comment

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

This is an API change, isn't it?

Copy link
Member

Choose a reason for hiding this comment

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

As are the added args below? Not sure if they are a problem, but prob needs an Api change note

Name of the tool to add, see `ToolManager.get_tool`.
group : str
The name of the group to add this tool to.
position : int, default: -1
The position within the group to place this tool.
"""
tool = self.toolmanager.get_tool(tool)
tool = self.toolmanager.get_tool(tool_name)
image = self._get_image_filename(tool.image)
toggle = getattr(tool, 'toggled', None) is not None
self.add_toolitem(tool.name, group, position,
self.add_toolitem(tool_name, group, position,
image, tool.description, toggle)
if toggle:
self.toolmanager.toolmanager_connect('tool_trigger_%s' % tool.name,
self.toolmanager.toolmanager_connect('tool_trigger_%s' % tool_name,
self._tool_toggled_cbk)
# If initially toggled
if tool.toggled:
self.toggle_toolitem(tool.name, True)
self.toggle_toolitem(tool_name, True)

def _get_image_filename(self, image):
"""Find the image based on its name."""
Expand Down
41 changes: 22 additions & 19 deletions lib/matplotlib/backend_managers.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,19 @@

class ToolEvent:
"""Event for tool manipulation (add/remove)."""
def __init__(self, name, sender, tool, data=None):
def __init__(self, name, sender, tool, tool_name, data=None):
self.name = name
self.sender = sender
self.tool = tool
self.tool_name = tool_name
self.data = data


class ToolTriggerEvent(ToolEvent):
"""Event to inform that a tool has been triggered."""
def __init__(self, name, sender, tool, canvasevent=None, data=None):
ToolEvent.__init__(self, name, sender, tool, data)
def __init__(self, name, sender, tool, tool_name,
canvasevent=None, data=None):
ToolEvent.__init__(self, name, sender, tool, tool_name, data)
self.canvasevent = canvasevent


Expand Down Expand Up @@ -234,7 +236,7 @@ def remove_tool(self, name):
self._remove_keys(name)

s = 'tool_removed_event'
event = ToolEvent(s, self, tool)
event = ToolEvent(s, self, tool, name)
self._callbacks.process(s, event)

del self._tools[name]
Expand Down Expand Up @@ -272,7 +274,7 @@ def add_tool(self, name, tool, *args, **kwargs):
'exists, not added')
return self._tools[name]

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

if tool_cls.default_keymap is not None:
Expand All @@ -289,24 +291,25 @@ def add_tool(self, name, tool, *args, **kwargs):

# If initially toggled
if tool_obj.toggled:
self._handle_toggle(tool_obj, None, None, None)
self._handle_toggle(name, tool_obj, None, None, None)
tool_obj.set_figure(self.figure)

self._tool_added_event(tool_obj)
self._tool_added_event(tool_obj, name)
return tool_obj

def _tool_added_event(self, tool):
def _tool_added_event(self, tool, tool_name):
s = 'tool_added_event'
event = ToolEvent(s, self, tool)
event = ToolEvent(s, self, tool, tool_name)
self._callbacks.process(s, event)

def _handle_toggle(self, tool, sender, canvasevent, data):
def _handle_toggle(self, tool_name, tool, sender, canvasevent, data):
"""
Toggle tools, need to untoggle prior to using other Toggle tool.
Called from trigger_tool.

Parameters
----------
tool_name: String
tool : `.ToolBase`
sender : object
Object that wishes to trigger the tool.
Expand All @@ -320,27 +323,27 @@ def _handle_toggle(self, tool, sender, canvasevent, data):
# radio_group None is not mutually exclusive
# just keep track of toggled tools in this group
if radio_group is None:
if tool.name in self._toggled[None]:
self._toggled[None].remove(tool.name)
if tool_name in self._toggled[None]:
self._toggled[None].remove(tool_name)
else:
self._toggled[None].add(tool.name)
self._toggled[None].add(tool_name)
return

# If the tool already has a toggled state, untoggle it
if self._toggled[radio_group] == tool.name:
if self._toggled[radio_group] == tool_name:
toggled = None
# If no tool was toggled in the radio_group
# toggle it
elif self._toggled[radio_group] is None:
toggled = tool.name
toggled = tool_name
# Other tool in the radio_group is toggled
else:
# Untoggle previously toggled tool
self.trigger_tool(self._toggled[radio_group],
self,
canvasevent,
data)
toggled = tool.name
toggled = tool_name

# Keep track of the toggled tool in the radio_group
self._toggled[radio_group] = toggled
Expand Down Expand Up @@ -387,15 +390,15 @@ def trigger_tool(self, name, sender=None, canvasevent=None, data=None):
self._trigger_tool(name, sender, canvasevent, data)

s = 'tool_trigger_%s' % name
event = ToolTriggerEvent(s, sender, tool, canvasevent, data)
event = ToolTriggerEvent(s, sender, tool, name, canvasevent, data)
self._callbacks.process(s, event)

def _trigger_tool(self, name, sender=None, canvasevent=None, data=None):
"""Actually trigger a tool."""
tool = self.get_tool(name)

if isinstance(tool, tools.ToolToggleBase):
self._handle_toggle(tool, sender, canvasevent, data)
self._handle_toggle(name, tool, sender, canvasevent, data)

# Important!!!
# This is where the Tool object gets triggered
Expand Down Expand Up @@ -434,7 +437,7 @@ def get_tool(self, name, warn=True):
`.ToolBase` or None
The tool or None if no tool with the given name exists.
"""
if isinstance(name, tools.ToolBase) and name.name in self._tools:
if isinstance(name, tools.ToolBase) and name in self._tools.values():
return name
if name not in self._tools:
if warn:
Expand Down
21 changes: 6 additions & 15 deletions lib/matplotlib/backend_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,6 @@ class ToolBase:
ToolManager that controls this Tool.
figure : `FigureCanvas`
Figure instance that is affected by this Tool.
name : str
Used as **Id** of the tool, has to be unique among tools of the same
ToolManager.
"""

default_keymap = None
Expand All @@ -79,11 +76,10 @@ class ToolBase:
*name* is used as a label in the toolbar button.
"""

def __init__(self, toolmanager, name):
def __init__(self, toolmanager):
cbook._warn_external(
'The new Tool classes introduced in v1.5 are experimental; their '
'API (including names) will likely change in future versions.')
self._name = name
self._toolmanager = toolmanager
self._figure = None

Expand Down Expand Up @@ -142,11 +138,6 @@ def trigger(self, sender, event, data=None):
"""
pass

@property
def name(self):
"""Tool Id."""
return self._name

def destroy(self):
"""
Destroy the tool.
Expand Down Expand Up @@ -262,8 +253,8 @@ def __init__(self, *args, **kwargs):
self._add_tool_cbk)

# process current tools
for tool in self.toolmanager.tools.values():
self._add_tool(tool)
for tool_name, tool in self.toolmanager.tools.items():
self._add_tool(tool_name, tool)

def set_figure(self, figure):
if self._id_drag:
Expand All @@ -281,17 +272,17 @@ def _tool_trigger_cbk(self, event):

self._set_cursor_cbk(event.canvasevent)

def _add_tool(self, tool):
def _add_tool(self, tool_name, tool):
"""Set the cursor when the tool is triggered."""
if getattr(tool, 'cursor', None) is not None:
self.toolmanager.toolmanager_connect('tool_trigger_%s' % tool.name,
self.toolmanager.toolmanager_connect('tool_trigger_%s' % tool_name,
self._tool_trigger_cbk)

def _add_tool_cbk(self, event):
"""Process every newly added tool."""
if event.tool is self:
return
self._add_tool(event.tool)
self._add_tool(event.tool_name, event.tool)

def _set_cursor_cbk(self, event):
if not event:
Expand Down
2 changes: 1 addition & 1 deletion lib/matplotlib/backends/backend_gtk3.py
Original file line number Diff line number Diff line change
Expand Up @@ -882,7 +882,7 @@ def _show_shortcuts_window(self):
self._normalize_shortcut(key)
for key in self.toolmanager.get_tool_keymap(name)
if self._is_valid_shortcut(key)),
title=tool.name,
title=name,
subtitle=tool.description)
group.add(shortcut)

Expand Down