-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
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
Changes from 1 commit
8cceed4
3118a5a
b4d5fcf
1e8af47
622cb95
d1a9de4
3f89d52
4f3c10b
6065daa
f6a2f19
05db3b6
c08fe56
b207a72
9266447
a53419a
704c717
5056729
e6a4e1e
8942c47
022de6f
2c9a195
cafe668
224f745
94c711e
67257e7
ffa65d6
6739ee0
d18206f
34a52c8
c2da483
44a9b0e
a2ed47f
0665890
411e6e2
d484ebd
75bf97b
6cc040b
0ff5997
af6734f
78513d2
377ff54
7dbbf58
dd66b57
67a414f
e415d8d
1213086
ba61dec
9f2ee2b
9da2b13
110253f
e2804ea
9a64b7e
64f947f
e8cd5d5
4bbcf4e
73a2661
1b83628
e4edd23
d4ac2fb
a7640ef
48a6971
8dafe09
a0695d0
328b169
aac4744
f09b9ef
def3a52
9ee7e25
5eae4e1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
import matplotlib | ||
# matplotlib.use('GTK3Cairo') | ||
matplotlib.use('TkAGG') | ||
matplotlib.use('GTK3Cairo') | ||
# matplotlib.use('TkAGG') | ||
matplotlib.rcParams['toolbar'] = 'navigation' | ||
import matplotlib.pyplot as plt | ||
from matplotlib.backend_tools import ToolBase | ||
|
@@ -53,7 +53,7 @@ def trigger(self, event): | |
if matplotlib.rcParams['backend'] == 'GTK3Cairo': | ||
fig.canvas.manager.navigation.add_tool('copy', CopyToolGTK3) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd be fine with making this a GTK3Cairo-only example. Makes it a little bit simpler |
||
|
||
# Just for fun, lets remove the back button | ||
# fig.canvas.manager.navigation.remove_tool('Back') | ||
# Just for fun, lets remove the forward button | ||
fig.canvas.manager.navigation.remove_tool('Forward') | ||
|
||
plt.show() |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3288,6 +3288,11 @@ def get_tool_keymap(self, name): | |
keys = [k for k, i in six.iteritems(self._keys) if i == name] | ||
return keys | ||
|
||
def _remove_keys(self, name): | ||
keys = [k for k, v in six.iteritems(self._keys) if v == name] | ||
for k in keys: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What do you think about replacing this with:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes. |
||
del self._keys[k] | ||
|
||
def set_tool_keymap(self, name, *keys): | ||
"""Set the keymap associated with a tool | ||
|
||
|
@@ -3301,9 +3306,7 @@ def set_tool_keymap(self, name, *keys): | |
if name not in self._tools: | ||
raise AttributeError('%s not in Tools' % name) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This isn't really an AttributeError, is it? Would seem more like a KeyError. |
||
|
||
active_keys = [k for k, i in six.iteritems(self._keys) if i == name] | ||
for k in active_keys: | ||
del self._keys[k] | ||
self._remove_keys(name) | ||
|
||
for key in keys: | ||
for k in validate_stringlist(key): | ||
|
@@ -3312,21 +3315,6 @@ def set_tool_keymap(self, name, *keys): | |
(k, self._keys[k], name)) | ||
self._keys[k] = name | ||
|
||
def unregister(self, name): | ||
"""Unregister the tool from Navigation | ||
|
||
Parameters | ||
---------- | ||
name : string | ||
Name of the tool to unregister | ||
""" | ||
|
||
if self._toggled == name: | ||
self._handle_toggle(name, from_toolbar=False) | ||
if name in self._tools: | ||
self._tools[name].destroy() | ||
del self._tools[name] | ||
|
||
def remove_tool(self, name): | ||
"""Remove tool from the `Navigation` | ||
|
||
|
@@ -3336,15 +3324,19 @@ def remove_tool(self, name): | |
Name of the Tool | ||
""" | ||
|
||
self.unregister(name) | ||
tool = self._tools[name] | ||
tool.destroy() | ||
|
||
keys = [k for k, v in six.iteritems(self._keys) if v == name] | ||
for k in keys: | ||
del self._keys[k] | ||
if self._toggled == name: | ||
self._handle_toggle(name, from_toolbar=False) | ||
|
||
if self.toolbar: | ||
self._remove_keys(name) | ||
|
||
if self.toolbar and tool.intoolbar: | ||
self.toolbar._remove_toolitem(name) | ||
|
||
del self._tools[name] | ||
|
||
def add_tools(self, tools): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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... There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 👎 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is more for backend implementation. It was done automatically at There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @OceanWolf where do you suggest to add that loop? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
""" Add multiple tools to `Navigation` | ||
|
||
|
@@ -3491,7 +3483,7 @@ def get_tools(self): | |
for name in sorted(self._tools.keys()): | ||
tool = self._tools[name] | ||
keys = [k for k, i in six.iteritems(self._keys) if i == name] | ||
d[name] = {'cls': tool, | ||
d[name] = {'obj': tool, | ||
'description': tool.description, | ||
'keymap': keys} | ||
return d | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -92,19 +92,6 @@ def set_figure(self, figure): | |
self.figure = figure | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should this be |
||
self.navigation = figure.canvas.manager.navigation | ||
|
||
def unregister(self, *args): | ||
"""Unregister the tool from the instances of Navigation | ||
|
||
It is usually called by during destroy if it is a | ||
graphical Tool. | ||
|
||
If the reference in navigation was the last reference | ||
to the instance of the tool, it will be garbage collected | ||
""" | ||
|
||
# call this to unregister from navigation | ||
self.navigation.unregister(self._name) | ||
|
||
@property | ||
def name(self): | ||
return self._name | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since these are examples, I would suggest getting rid of extraneous commented out code, and also highlight important lines of code such as this one. For example, is it important that it gets called before importing pyplot? What is it for? Perhaps a short docstring at the top of this example would help explain its purpose/goal that it is demonstrating?