-
-
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
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 |
---|---|---|
|
@@ -3421,6 +3421,9 @@ def add_tool(self, tool): | |
""" | ||
|
||
tool_cls = self._get_cls_to_instantiate(tool) | ||
if tool_cls is False: | ||
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. probably should do a 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. Sorry, I don't see the pep8 error here. 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 think this should be Wouldn't be simpler to just have I suspect @WeatherGod also has a pyflake/pylint turned on. 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. also, I am reading and commenting top-down. Sorry if some of these seem dumb in light of code I have not read yet. 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 @tacaswell change, but still not see the pep8 error 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. Different versions of pep8? I find myself curious to learn about this as Travis doesn't complain. |
||
warnings.warn('Impossible to find class for %s' % str(tool)) | ||
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. Shouldn't this raise an exception? The function failed to achieve its goal and no new tool was added. |
||
return | ||
name = tool_cls.name | ||
|
||
if name is None: | ||
|
@@ -3442,9 +3445,11 @@ def add_tool(self, tool): | |
self._keys[k] = name | ||
|
||
if self.toolbar and tool_cls.position is not None: | ||
# TODO: better search for images, they are not always in the | ||
# datapath | ||
basedir = os.path.join(rcParams['datapath'], 'images') | ||
if tool_cls.image is not None: | ||
6D47 | fname = os.path.join(basedir, tool_cls.image + '.png') | |
fname = os.path.join(basedir, tool_cls.image) | ||
else: | ||
fname = None | ||
toggle = issubclass(tool_cls, tools.ToolToggleBase) | ||
|
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?