Skip to content

Commit

Permalink
Merge pull request #4219 from jasonliw93/master
Browse files Browse the repository at this point in the history
ENH : check existence of arbitrarily named figures

Closes #2880
  • Loading branch information
tacaswell committed Mar 15, 2015
2 parents c90469b + cd08a19 commit 0346946
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 1 deletion.
7 changes: 7 additions & 0 deletions doc/users/whats_new/updated_pyplot.rst
@@ -0,0 +1,7 @@
Updated fignum_exists to take figure name
-------------------------------------------
Added the ability to check the existence of a figure using it's name
instead of just the figure number.
Example:
figure('figure')
fignum_exists('figure') #true
4 changes: 3 additions & 1 deletion lib/matplotlib/pyplot.py
Expand Up @@ -460,7 +460,9 @@ def gcf():
else:
return figure()

fignum_exists = _pylab_helpers.Gcf.has_fignum
def fignum_exists(num):
allLabels = get_figlabels()
return _pylab_helpers.Gcf.has_fignum(num) or num in allLabels

This comment has been minimized.

Copy link
@lebigot

lebigot Mar 16, 2015

Contributor

Why create an intermediate variable allLabels for something which is used once, and more importantly, why call get_figlabels() when it can be unnecessary (because the or can match the old _pylab…()). I am not submitting a patch because there might be a reason that you experts out there can immediately find. :) Unless there is some side-effect to the get_figlabels() call, this would definitely be better as return _pylab_helpers.Gcf.has_fignum(num) or num in get_figlabels() (with no allLabels variable).

This comment has been minimized.

Copy link
@tacaswell

tacaswell Mar 16, 2015

Author Member

No reason other than readability. Is this really a performance bottleneck?

This comment has been minimized.

Copy link
@lebigot

lebigot Mar 16, 2015

Contributor

OK, then. :) We do have a different feeling about readability, though: fewer variables make the code easier to fit in one's head, as a general principle (not that it matters much here). Similarly, I always prefer code that is more efficient when it costs nothing (or is even better) to write it this way (not that it matter much here either). As a consequence, when I read this code, I also wonder why not do something slightly less taxing on the memory, slightly more efficient, and shorter: I even start to wonder if get_figLabels() has some side effect and can explain the style choice made here. So, this is not a big deal in practice, but I like the idea that Matplotlib be closer to a "beautiful chest of drawers", and the "wood" I would use for its back is quite different from this—I am not judging, just wanting to explain what I would do, and why. :) If I were to contribute to Matplotlib's codebase, this kind of coding would make me spend much more time than needed (because I would check that get_figLabels() has no side effect with an impact here).

This comment has been minimized.

Copy link
@tacaswell

tacaswell Mar 16, 2015

Author Member

Fair enough. I tend to err on the side of a larger number of simpler lines. Suppressing variables doesn't get rid of the complexity, it just makes it implicit.

If you put in a PR to make this change I will merge it.



def get_fignums():
Expand Down
17 changes: 17 additions & 0 deletions lib/matplotlib/tests/test_figure.py
Expand Up @@ -32,6 +32,23 @@ def test_figure_label():
assert_equal(plt.get_figlabels(), ['', 'today'])


@cleanup
def test_fignum_exists():
# pyplot figure creation, selection and closing with fignum_exists
plt.figure('one')
plt.figure(2)
plt.figure('three')
plt.figure()
assert_equal(plt.fignum_exists('one'), True)
assert_equal(plt.fignum_exists(2), True)
assert_equal(plt.fignum_exists('three'), True)
assert_equal(plt.fignum_exists(4), True)
plt.close('one')
plt.close(4)
assert_equal(plt.fignum_exists('one'), False)
assert_equal(plt.fignum_exists(4), False)


@image_comparison(baseline_images=['figure_today'])
def test_figure():
# named figure support
Expand Down

0 comments on commit 0346946

Please sign in to comment.