Bug summary

When removing a tool from the toolmanager, if the tool is not also present on the toolbar, it triggers a KeyError exception. It does remove the tool, however.

Code for reproduction

import matplotlib.pyplot as plt
plt.rcParams['toolbar'] = 'toolmanager'

fig = plt.figure()
tm = fig.canvas.manager.toolmanager

# Removing a tool that is on the toolbar causes no issue
tm.remove_tool("zoom")

# However, removing a tool that is not on the toolbar does results in an exception
tm.remove_tool("grid")

Actual outcome

Traceback (most recent call last):
File "...\matplotlib\cbook.py", line 298, in process
func(*args, **kwargs)
File "...\matplotlib\backend_bases.py", line 3234, in
lambda event: self.remove_toolitem(event.tool.name))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "...\matplotlib\backends\backend_qt.py", line 964, in remove_toolitem
for button, handler in self._toolitems[name]:
~~~~~~~~~~~~~~~^^^^^^
KeyError: 'grid'

Expected outcome

Should not raise any exception

Additional information

It does perform the necessary operations to remove the tool from the toolmanager, then tries to remove it from the toolbar as well. This second step is what fails.

As I understand, the remove_tool method of the Toolmanager class (in file backend_managers) fires a "tool_removed_event" after having deleted the tool from the toolmanager:

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

This in turns triggers the remove_toolitem method of the Toolbar class, which reads (in file backend_qt, but it is the same in the other backends that I have checked):

    def remove_toolitem(self, name):
        for button, handler in self._toolitems[name]:
            button.setParent(None)
        del self._toolitems[name]

name is the name of the tool. The first line raises an exception because it is not present in the toolbar and thus it is not a key of self._toolitems

I dont know what the best practice would be, and I'm no formally trained developper, but one option to solve the problem is to catch the exception in this last method (remove_toolitem). Perhaps something like that:

    def remove_toolitem(self, name):
        if name not in self._toolitems.keys():
            return
        for button, handler in self._toolitems[name]:
            button.setParent(None)
        del self._toolitems[name]

or

    def remove_toolitem(self, name):
        try:
            toolitem = self._toolitems[name]
        except KeyError:
            return
        for button, handler in toolitem:
            button.setParent(None)
        del self._toolitems[name]

The drawback is that it would have to be changed for the different backends (I see at least backend_qt.py, _backend_tk.py, backend_wx.py). I believe that going one step higher in the class hierarchy to fix it for all backends at once would be significantly more involved. Indeed, the ToolContainerBase class (backend_bases file), from which the Toolbar class for the different backends inherits, connects the "tool_removed_event" to the "remove_toolitem" method in its init:

        toolmanager.toolmanager_connect(
            'tool_removed_event',
            lambda event: self.remove_toolitem(event.tool.name))

As far as I understand, there is no way to know, at this point, whether the tool of interest is indeed "registered" in the toolbar because each backend is free to implement "tool storage" as it pleases.

Operating system

Windows

Matplotlib Version

3.8 (I checked that the problematic code is identical in the newest version)

Matplotlib Backend

Qt5Agg and Tkinter (checked both)

Python version

3.11

Jupyter version

No response

Installation

None