Help tool #9022

fariza · 2017-08-11T22:13:21Z

This PR adds a tool to show the usable tools and it's shortcuts (keymap).

As most people are used to press F1 to bring up some help menu or information f1 was selected as shortcut

This 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?

afvincent · 2017-08-11T22:26:05Z

@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 (I never used it so I am not sure how relevant it could be)?

WeatherGod · 2017-08-12T00:31:55Z

I love the idea, but it does need refinement. May I suggest a question mark icon?

…

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> .

fariza · 2017-08-12T17:42:23Z

@WeatherGod question mark in the toolbar? or as shortcut?

fariza · 2017-08-12T18:29:22Z

Using monospace as suggested (thanks @afvincent ), the alignment problem is gone

Before pressing f1

After pressing f1

fariza · 2017-08-12T18:50:53Z

Here is with the help icon

WeatherGod · 2017-08-13T04:15:44Z

Thanks for the icon. Now, would it make sense to put the keymap column after the name column? I would usually look for a tool by name and want to know their shortcut, not their 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> .

story645 · 2017-08-13T05:17:50Z

I think this is a great idea, but I'm a little confused by Name(id) and the Active Toggle Tools header. I'm unsure if Name is the same as id and if there's a reason I need to know that, and I wonder if `Active Toggle Tools can be dropped since right underneath it says "Group" and "Active"-and what sort of group membership is that?

fariza · 2017-08-14T13:05:11Z

@story645 This tool is for the new toolmanager.
The printout is the same from the toolmanager example, that is why it has the active toggle tools, remember toggle tools can belong to a group where they are mutually exclusive (radio button), that is why it shows the current toggled tool for each group.
I will remove that group part as it is not really relevant to the "Help"

fariza · 2017-08-15T19:43:14Z

Removing the toggle groups and keymap before description

anntzer · 2017-08-19T03:05:27Z

What are the "List" and "Show" tools that appear at the top?

fariza · 2017-08-19T03:25:45Z

@anntzer The screenshot is from the ToolManager example. That is why those two tools are there (that particular example custom tools)

anntzer · 2017-08-19T03:28:02Z

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.

anntzer · 2017-08-19T03:29:58Z

lib/matplotlib/backend_tools.py

+    def get_text(self):
+        sep = '_' * 80 + '\n'
+        t = sep
+        t += "{0:15} {1:25} {2}\n".format(


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).

I'm pretty bad with fonts, if I keep using monospace, how can I know what is the "length" (in characters) of the canvas?

I mean replace 15 and 25 by whatever number is necessary (just count the characters of the longest string).

I understand, but wouldn't it be better to make it to adjust to the number of characters that fit into the "width"

It's monospace so by definition all characters have the same width.

Yes, that I know. But don't size 20 what does it mean in terms of width?

I am no expert of fonts, but I think there is no way to precisely know that in a simple way (i.e. outside of the "font engine"). However some SO 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 ^^)

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).

fariza · 2017-08-21T00:17:43Z

@anntzer the warning is in place from the begining.
If you run the example examples/user_interfaces/toolmanager_sgskip.py

you will get a warning
workspace/matplotlib/lib/matplotlib/backend_managers.py:209: UserWarning: Key G changed from grid_minor to Show (k, self._keys[k], name))

fariza · 2017-09-06T23:48:21Z

It took me some time, but I decided to try to guess the number of characters per line (monospace) using the approx 60% (thanks for the tip).

Here is how it looks now, using this approach

issues addressed

anntzer · 2017-09-07T00:07:19Z

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",
and the inner caps in Toggle Fullscreen mode and Toggle Scale X/Y removed (for consistency with the other descriptions). There's also a typo Toogle -> Toggle (multiple times).

fariza · 2017-09-07T18:35:23Z

@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.
Nav keys don't fit into a sinlge line, but it is exceptionally long

anntzer · 2017-09-07T18:53:50Z

lib/matplotlib/backend_tools.py

@@ -14,6 +14,8 @@

 from matplotlib import rcParams
 from matplotlib._pylab_helpers import Gcf
+from matplotlib.table import Table
+import textwrap


move the import down per PEP8

anntzer · 2017-09-07T18:55:27Z

lib/matplotlib/backend_tools.py

+        self.text_axes = None
+
+    def enable(self, *args):
+        self.text_axes = self.figure.add_axes((0, 0, 1, 1))


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.

anntzer · 2017-09-07T18:55:51Z

few minor things, but LGTM otherwise.

fariza · 2018-02-26T17:28:33Z

Is there something left to do for this PR?

fariza · 2018-04-09T18:55:46Z

@anntzer @afvincent Is there something missing?

ImportanceOfBeingErnest · 2018-04-09T19:29:25Z

lib/matplotlib/backend_tools.py

+
+    def disable(self, *args):
+        self.text_axes.remove()
+        self.figure.canvas.draw_idle()


Does this mean that after having pressed that button 10 times, you have 10 unused Axes in memory? Should it maybe rather reuse an existing axes, or comletely delete the unused axes?

when creating the axes we specify the label, so if I understand it correctly it should reuse it if available

That behavior (of reusing axes) is deprecated.

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.)

Create it once, call set_visible(True), call set_visible(False).

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

Setting the visibility lets the axes stay in fig.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.

anntzer · 2018-04-09T20:05:07Z

See #10851 (comment).

ImportanceOfBeingErnest · 2018-04-12T21:06:53Z

There might be another problem with this approach of adding an axes. I think if you happen to have set fig.set_tight_layout(True) and then enter help mode, it would trigger a warning about the axes not being compatible with tight_layout.

timhoffm · 2018-04-12T22:39:09Z

Not quite sure if it is generally a good idea to do this within the figure.

Advantage: Works for all backends.
Disadvantage: May mess up an existing plot.

Alternatives:

Backend-native popup window. More work (i.e. per backend), but completely decoupled from the drawing logic.
Compromise: Show this in a separate figure.

anntzer · 2018-04-13T07:54:45Z

fwiw I am pretty strongly in favor of using backend-native stuff whenever possible (and yes, I know that @fariza thinks the opposite :-))...

anntzer · 2018-04-13T07:56:54Z

lib/matplotlib/backend_tools.py

@@ -1020,6 +1021,75 @@ def _mouse_move(self, event):
        self.toolmanager.canvas.draw_idle()


+class HelpTool(ToolToggleBase):


Should be ToolHelp for consistency (or rather all others should be FooTool, but heh).

ImportanceOfBeingErnest · 2018-04-13T09:24:49Z

Creating a separate figure is really easy if one can rely on using pyplot for that,

self.auxfig = pyplot.figure()
# ....
self.auxfig.show()

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 set_text and get_figure method.
This could then be used by the helper tool, but equally by all any other tool that needs some window.
Of course it would need to be implemented by all backends and individual backends could implement further methods if needed.
Once such a window exists, the creation of new tools which need a separate figure or just some popup window would be really easy.

fariza · 2018-04-13T12:26:10Z

My problem with specific backend stuff is that we have to support it. It looks better I agree. It is easy to create one window, set the title, set the size plug the callbacks, 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 end with too many windows creatio/control to do it efficiently. That is why MEP27 exists and why we have been trying to get it properly reviewed for years. We want to do that window creation process for each backend once AND only once. Then implement generic tools (as much as possible) 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> .

fariza · 2018-04-13T18:06:12Z

@ImportanceOfBeingErnest we don't want to use pyplot, that is something that we want removed somewhere in the future.

timhoffm · 2018-04-16T11:11:42Z

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:

Either use this code but display it in a separate figure. For the time being, one can rely on pyplot for handling the backend-specific stuff for figure creation. Once MEP27 gets accepted, we can migrate the help from pyplot to the new API.
Or, use the native widgets Help tool. #11045. As long as there is not more than the help window, that's fine in terms of complexity. If you want to do it for 10 different things, one will have to think about internal backend abstractions for simple general-purpose windows. I assume that you would want some interactive elements like buttons etc. as well, which is also not easy when drawing it all on a figure.

I tend to go with the native widgets.

tacaswell added this to the 2.1 (next point release) milestone Aug 13, 2017

anntzer previously requested changes Aug 19, 2017

View reviewed changes

fariza mentioned this pull request Aug 23, 2017

removing import that is prone to circular imports #9079

Merged

6 tasks

fariza force-pushed the help-tool branch from 0c27fc7 to 5773918 Compare September 6, 2017 23:44

anntzer reviewed Sep 7, 2017

View reviewed changes

tacaswell modified the milestones: 2.1 (next point release), 2.2 (next next feature release) Sep 24, 2017

tacaswell modified the milestones: needs sorting, v3.0 Feb 26, 2018

fariza added the status: needs review label Mar 12, 2018

ImportanceOfBeingErnest reviewed Apr 9, 2018

View reviewed changes

jklymak added the status: needs rebase label Apr 9, 2018

Federico Ariza and others added 5 commits April 12, 2018 15:53

adding help tool

38d83af

more space for keys

137df89

grammar/ortograph

2ce02b4

pep8 imports and unique axes with custom label

a06a80b

add help entry to matplotlibrc.template

54cf09f

fariza force-pushed the help-tool branch from c32a7d2 to 54cf09f Compare April 12, 2018 20:07

fariza removed the status: needs rebase label Apr 12, 2018

reusing the help tool axes

b89a904

anntzer reviewed Apr 13, 2018

View reviewed changes

anntzer mentioned this pull request Apr 13, 2018

Help tool. #11045

Merged

6 tasks

fariza closed this Apr 18, 2018

fariza deleted the help-tool branch April 18, 2018 18:21

timhoffm removed the status: needs review label Apr 18, 2018

		@@ -1020,6 +1021,75 @@ def _mouse_move(self, event):
		self.toolmanager.canvas.draw_idle()


		class HelpTool(ToolToggleBase):

Help tool #9022

Help tool #9022

Conversation

fariza commented Aug 11, 2017

afvincent commented Aug 11, 2017

WeatherGod commented Aug 12, 2017 via email

fariza commented Aug 12, 2017

fariza commented Aug 12, 2017

fariza commented Aug 12, 2017

WeatherGod commented Aug 13, 2017 via email

story645 commented Aug 13, 2017

fariza commented Aug 14, 2017

fariza commented Aug 15, 2017

anntzer commented Aug 19, 2017

fariza commented Aug 19, 2017

anntzer commented Aug 19, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fariza commented Aug 21, 2017

fariza commented Sep 6, 2017

anntzer commented Sep 7, 2017

fariza commented Sep 7, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anntzer commented Sep 7, 2017

fariza commented Feb 26, 2018

fariza commented Apr 9, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anntzer commented Apr 9, 2018

ImportanceOfBeingErnest commented Apr 12, 2018

timhoffm commented Apr 12, 2018

anntzer commented Apr 13, 2018

Choose a reason for hiding this comment

ImportanceOfBeingErnest commented Apr 13, 2018

fariza commented Apr 13, 2018 via email

fariza commented Apr 13, 2018

timhoffm commented Apr 16, 2018

anntzer commented Aug 19, 2017 •

edited

Loading