FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Named Figures feature: plot('today') by butterw · Pull Request #147 · matplotlib/matplotlib · GitHub

Repository navigation

Named Figures feature: plot('today') - #147

Merged
efiring merged 3 commits into
matplotlib:masterfrom
butterw:master
Aug 20, 2011
Merged

efiring merged 3 commits into
matplotlib:masterfrom
butterw:master

Conversation

butterw commented Jun 17, 2011

Copy link
Copy Markdown

Continued from issue #100 (my repo was messed up).

Hopefully this time it works...

ddale commented Jun 18, 2011

Copy link
Copy Markdown
Contributor

I don't think the figure name should appear in the canvas. Naming a figure is not the same thing as giving a figure or axes a title.

butterw commented Jun 19, 2011

Copy link
Copy Markdown
Author

This is done only in the image comparison test.
The aim of that test is to check that plotting works ok with named figures.
To ensure plotting is done on the correct figure, it is useful to draw the
figure name on the output.

Le , ddale
reply@reply.github.com
a crit :

I don't think the figure name should appear in the canvas. Naming a
figure is not the same thing as giving a figure or axes a title.

Reply to this email directly or view it on GitHub:

#147 (comment)

Comment thread lib/matplotlib/pyplot.py Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I don't think the logic here is correct. There is no guarantee that the index of figLabel corresponds to the figure number.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

should be:
num = allLabels.index(figLabel)
num = allnums[num]

Copy link
Copy Markdown
Contributor

To elaborate my comment on the source code, here is an example

import matplotlib.pyplot as plt
f1 = plt.figure("1")
f2 = plt.figure("2")
f3 = plt.figure("3")
   
plt.close("1")
from matplotlib.pyplot import _pylab_helpers
figManagers = _pylab_helpers.Gcf.get_all_fig_managers()
print [m.num for m in figManagers]

And it prints out

[2, 3]

We may use a dictionary that maps labels to figure numbers.

butterw commented Jun 19, 2011

Copy link
Copy Markdown
Author

There was a mistake in the code (corrected below).

I don't think Dictionnary access is possible because labels are not guaranteed to be unique.

correct version:
if figLabel not in allLabels:
if len(allLabels):
num = max(allnums) + 1
else:
num = 1
else:
num = allLabels.index(figLabel)
num = allnums[num]

Copy link
Copy Markdown
Contributor

Please push your changes into your repo.

Eric commented on the issue of using "all" as a figure name (#99 (comment)). And I think it is better to explicitly prevent it from being used.

butterw commented Jun 20, 2011

Copy link
Copy Markdown
Author

What is the desired behavior when a new figure('all') is created ?

  1. create figure
  2. create figure + warn
  3. do not create figure + warn

I think 'all' is a reasonable value for creating/selecting a named figure. It's only if close('all') is called that there is a possible issue. Therefore, I would go with solution 2).

the warning message could be the following:
warning: using figure name 'all' is not recommended because calling close('all') will close all existing figures instead of just figure 'all'. figure('all') was created.

Copy link
Copy Markdown
Contributor

Okay. With a reasonable warning message, option 2 should be good.

butterw commented Jul 14, 2011

Copy link
Copy Markdown
Author

The feature is fully implemented and ready for pull.

efiring added a commit that referenced this pull request Aug 20, 2011
Named Figures feature: plot('today')
efiring merged commit 94cf18d into matplotlib:master Aug 20, 2011

efiring commented Aug 20, 2011

Copy link
Copy Markdown
Member

This seems to me like a useful and logical feature, and a bit of testing turned up no surprises, so I saw no reason not to merge it.

richbwood mentioned this pull request Dec 19, 2012
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants


Back | FazBrowse Home | New Git URL