Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Help tool#9022
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Help tool#9022
Uh oh!
There was an error while loading.Please reload this page.
Conversation
I love the idea, but it does need refinement. May I suggest a question markicon? …On Aug 11, 2017 6:26 PM, "Adrien F. Vincent" ***@***.***> wrote:@fariza <https://github.com/fariza> (Very) Dummy ideas about the text misalignement: could forcing a monospace font be of any help (but yes it is kind of a nuke...) or maybe have a look ax.table <https://matplotlib.org/api/_as_gen/matplotlib.axes.Axes.table.html> (I never used it so I am not sure how relevant it could be)? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#9022 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AARy-AVjcmg-DghKZm0aDxA31Yn8dUJwks5sXNT-gaJpZM4O1L_t> . |
@WeatherGod question mark in the toolbar? or as shortcut? |
Using monospace as suggested (thanks@afvincent ), the alignment problem is gone |
Thanks for the icon.Now, would it make sense to put the keymap column after the name column? Iwould usually look for a tool by name and want to know their shortcut, nottheir full description. …On Aug 12, 2017 2:50 PM, "Federico Ariza" ***@***.***> wrote: Here is with the help icon [image: screenshot from 2017-08-12 14-46-03] <https://user-images.githubusercontent.com/1490153/29243462-9de13a98-7f6d-11e7-8884-5ad9aac89d86.png> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#9022 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AARy-GvmfuYCh-Q3vBjPN8YVnSFTD4k-ks5sXfQOgaJpZM4O1L_t> . |
I think this is a great idea, but I'm a little confused by |
@story645 This tool is for the new toolmanager. |
What are the "List" and "Show" tools that appear at the top? |
@anntzer The screenshot is from the ToolManager example. That is why those two tools are there (that particular example custom tools) |
anntzer commentedAug 19, 2017 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
That example should probably be updated to use another keymap than G, which is now used by default by grid_minor. Edit: Also this kind of collision should probably be detected and trigger an error or at least a warning from toolmanager. |
lib/matplotlib/backend_tools.py Outdated
def get_text(self): | ||
sep = '_' * 80 + '\n' | ||
t = sep | ||
t += "{0:15} {1:25} {2}\n".format( |
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.
The column and separator widths should get computed from the strings that will actually be used, instead of being hardcoded. Same comment applies below.
Or the descriptions and keys should get textwrapped (https://docs.python.org/3/library/textwrap.html#textwrap.wrap).
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.
I'm pretty bad with fonts, if I keep using monospace, how can I know what is the "length" (in characters) of the canvas?
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.
I mean replace 15 and 25 by whatever number is necessary (just count the characters of the longest string).
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.
I understand, but wouldn't it be better to make it to adjust to the number of characters that fit into the "width"
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.
It's monospace so by definition all characters have the same width.
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.
Yes, that I know. But don't size 20 what does it mean in terms of width?
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.
I am no expert of fonts, but I think there is no way toprecisely know that in asimple way (i.e. outside of the "font engine"). However someSO sources (here is an archive of the mentioned original source) suggest sthat a 60 % width-to-height ratio may be a quite good approximation for monospace fonts.
So if I am not too wrong, a formula like:
Nmax of chars = width in inches / ((0.6 + epsilon) * fontsize in points * 1/72)
may be a rather easy way to estimate the number of characters that should fit in a given width. (PS: in case it is not clear, the PostScript point is 1/72 of international inch)
Anyway, AFAIU@anntzer´s suggestion, his idea was more to use columns of minimal width and that do not wrap the text (which may be easier to implement ^^)
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.
Yes, my point was as described by@afvincent.
If you really care about physical text size I think you need to use backend.get_text_width_height_descent but then you may as well look into how text wrapping is implemented (and not use a monospace font, but a plt.table).
@anntzer the warning is in place from the begining. you will get a warning |
Looks like the first column is too wide and the second one not wide enough? (the nav keys could definitely fit on a single line) As a side note, the description for allnav and nav should be changed to "Enable" instead of "Enables", |
@anntzer I changed to a 6 column setup 1,2,3 widths so, the second is the double of the first and the third is half the total. |
lib/matplotlib/backend_tools.py Outdated
@@ -14,6 +14,8 @@ | |||
from matplotlib import rcParams | |||
from matplotlib._pylab_helpers import Gcf | |||
from matplotlib.table import Table | |||
import textwrap |
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.
move the import down per PEP8
lib/matplotlib/backend_tools.py Outdated
self.text_axes = None | ||
def enable(self, *args): | ||
self.text_axes = self.figure.add_axes((0, 0, 1, 1)) |
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.
I think you should create the axes with a custom (internal) label (with a comment) to workaround#9024 (although that bad behavior is now deprecated), just in case.
few minor things, but LGTM otherwise. |
Is there something left to do for this PR? |
@anntzer@afvincent Is there something missing? |
lib/matplotlib/backend_tools.py Outdated
def disable(self, *args): | ||
self.text_axes.remove() | ||
self.figure.canvas.draw_idle() |
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.
Does this mean that after having pressed that button 10 times, you have 10 unusedAxes
in memory? Should it maybe rather reuse an existing axes, or comletely delete the unused axes?
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.
when creating the axes we specify the label, so if I understand it correctly it should reuse it if available
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.
That behavior (of reusing axes) is deprecated.
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.
But's it's not available, because you removed it from the figure. And if you didn't remove it, it would cause a warning, telling you that creating an instance with the exact same arguments will return a new instance in the future.
Simple test case:
fig = plt.figure()ax1 = fig.add_axes((0, 0, 1, 1), label='help_tool_axes')ax1.remove()ax2 = fig.add_axes((0, 0, 1, 1), label='help_tool_axes')print(ax1==ax2) # This will print False.
I think it would indeed be a good idea to reuse the axes, but I currently would not know how to do that. I.e. how would one add an axes to a figure which has previously been removed? (fig.add_axes(ax)
does not seem to work.)
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.
Create it once, callset_visible(True)
, callset_visible(False)
.
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.
Done, now it creates it once, set the visibility to change.
I clear the axes before filling it because the tools are dynamic, so the content might change between different calls
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.
Setting the visibility lets the axes stay infig.axes
. This might change any program's complete logic depending on whether or not you are using the ToolManager to show the figure and whether or not you have previously toggled the help or not.
Simple example:
for ax in fig.axes: # do something.
See#10851 (comment). |
There might be another problem with this approach of adding an axes. I think if you happen to have set |
Not quite sure if it is generally a good idea to do this within the figure. Advantage: Works for all backends. Alternatives:
|
fwiw I am pretty strongly in favor of using backend-native stuff whenever possible (and yes, I know that@fariza thinks the opposite :-))... |
@@ -1020,6 +1021,75 @@ def _mouse_move(self, event): | |||
self.toolmanager.canvas.draw_idle() | |||
class HelpTool(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.
Should be ToolHelp for consistency (or rather all others should be FooTool, but heh).
Creating a separate figure is really easy if one can rely on using pyplot for that,
Would there be a reason for this not to be the case? For the other proposal, maybe there should be a general pupose popup child window in the toolmanager which provides a |
My problem with specific backend stuff is that we have to support it. Itlooks better I agree.It is easy to create one window, set the title, set the size plug thecallbacks, etc.. but then doing it for 5 or 6 backends becomes cumbersome.And then if you want to do it for 1, 2 or 10 different tools you will endwith too many windows creatio/control to do it efficiently.That is why MEP27 exists and why we have been trying to get it properlyreviewed for years. We want to do that window creation process for eachbackend once AND only once. Then implement generic tools (as much aspossible) that can be shared and maintained easily …On Fri, Apr 13, 2018, 3:55 AM Antony Lee, ***@***.***> wrote: fwiw I am pretty strongly in favor of using backend-native stuff whenever possible (and yes, I know that@fariza <https://github.com/fariza> thinks the opposite :-))... — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#9022 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABa86akUWkz6WzYgyyGwJJC_ado_rlpZks5toFnggaJpZM4O1L_t> . |
@ImportanceOfBeingErnest we don't want to use pyplot, that is something that we want removed somewhere in the future. |
Not having read through the details of MEP27, but that seems to only target the plot window, not general purpose windows for displaying help. I see two possible strategies:
I tend to go with the native widgets. |
This PR adds a tool to show the usable tools and it's shortcuts (keymap).
As most people are used to pressF1 to bring up some help menu or information
f1
was selected as shortcutThis tool creates a new axes that expands to the whole figure (to hide everything), and add text with the current tool information, toggling again removes the axes

Currently there is a problem with alignment.
Does anybody has a better idea on how to generate the text to avoid this misalignment?