Repository navigation
scrub kwarg and use one figure for all of the tests #56
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
Show file tree
Hide file tree
Changes from all commits
File filter
Filter by extension
Filter by extension
.py
(11)
.rst
(1)
.template
(1)
No extension
(1)
All 4 file types selected
Viewed files
Viewed files
There are no files selected for viewing
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
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
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| Expand Up | @@ -21,7 +21,7 @@ def test_formatter_ticker(): | |
| ydata1 = [ (1.5*y - 0.5)*units.km for y in range(10) ] | ||
| ydata2 = [ (1.75*y - 1.0)*units.km for y in range(10) ] | ||
|
|
||
| fig = plt.figure(1);plt.clf() | ||
|
Comment thread
Copy link
Copy Markdown
Member
There was a problem hiding this comment. I don't like the duplication of this code all over the file. How about defining a helper function that calls plt.figure(1) and plt.clf(), and call it from all these sites?
Sorry, something went wrong.
Copy link
Copy Markdown
Member
Author
There was a problem hiding this comment. That would work, and would have the added benefit of making things more DRY with future such changes appearing only in one place. On the other hand, it would add a level of indirection that make it harder for someone looking at the tests who isn't that familiar with the codebase, to understand the distinction between what's in "pure" matplotlib, and the test suite itself. As an alternative to these changes, I also have an ivanov/close-all branch which sidesteps having to call clear due to the resuse of figure(1) by calling plt.close() after every test. This also keeping as much of the test code in "pure" matplotlib.
Sorry, something went wrong.
Copy link
Copy Markdown
Member
There was a problem hiding this comment. This is the crux of this patch -- on the one hand, calling plt.close() after every test is probably more what we want to recommend to end users. However, if I recall correctly, plt.clf() was much faster (and we have a lot of tests to run). @ivanov: do you remember how much faster?
Sorry, something went wrong.
Copy link
Copy Markdown
Member
Author
There was a problem hiding this comment. my notes say that it was 43 seconds faster to keep one figure around (522 with figure(1)+clf(), 585 with close() after every test), but I think this was before we tracked down some of the other memory leaks, so I'll try to rerun it again)
Sorry, something went wrong.
Copy link
Copy Markdown
Member
Author
There was a problem hiding this comment. oh, and I think the other problem with close() that we didn't end up fully catching is that IIRC we end up using more memory for the entire test suite (in other words, calling close() doesn't let go of some memory)
Sorry, something went wrong. |
||
| ax = plt.subplot( 111 ) | ||
| ax.set_xlabel( "x-label 001" ) | ||
| fig.savefig( 'formatter_ticker_001' ) | ||
| Expand Down Expand Up | @@ -50,7 +50,7 @@ def test_basic_annotate(): | |
|
|
||
| # Offset Points | ||
|
|
||
| fig = plt.figure(1);plt.clf() | ||
| ax = fig.add_subplot( 111, autoscale_on=False, xlim=(-1,5), ylim=(-3,5) ) | ||
| line, = ax.plot( t, s, lw=3, color='purple' ) | ||
|
|
||
| Expand All | @@ -75,7 +75,7 @@ def test_polar_annotations(): | |
| r = np.arange(0.0, 1.0, 0.001 ) | ||
| theta = 2.0 * 2.0 * np.pi * r | ||
|
|
||
| fig = plt.figure(1);plt.clf() | ||
| ax = fig.add_subplot( 111, polar=True ) | ||
| line, = ax.plot( theta, r, color='#ee8d18', lw=3 ) | ||
|
|
||
| Expand Down Expand Up | @@ -103,7 +103,7 @@ def test_polar_coord_annotations(): | |
| from matplotlib.patches import Ellipse | ||
| el = Ellipse((0,0), 10, 20, facecolor='r', alpha=0.5) | ||
|
|
||
| fig = plt.figure(1);plt.clf() | ||
| ax = fig.add_subplot( 111, aspect='equal' ) | ||
|
|
||
| ax.add_artist( el ) | ||
| Expand Down Expand Up | @@ -135,7 +135,7 @@ def test_fill_units(): | |
| value = 10.0 * units.deg | ||
| day = units.Duration( "ET", 24.0 * 60.0 * 60.0 ) | ||
|
|
||
| fig = plt.figure(1);plt.clf() | ||
|
|
||
| # Top-Left | ||
| ax1 = fig.add_subplot( 221 ) | ||
| Expand Down Expand Up | @@ -167,7 +167,7 @@ def test_fill_units(): | |
|
|
||
| @image_comparison(baseline_images=['single_point']) | ||
| def test_single_point(): | ||
| fig = plt.figure(1);plt.clf() | ||
| plt.subplot( 211 ) | ||
| plt.plot( [0], [0], 'o' ) | ||
|
|
||
| Expand All | @@ -181,7 +181,7 @@ def test_single_date(): | |
| time1=[ 721964.0 ] | ||
| data1=[ -65.54 ] | ||
|
|
||
| fig = plt.figure(1);plt.clf() | ||
| plt.subplot( 211 ) | ||
| plt.plot_date( time1, data1, 'o', color='r' ) | ||
|
|
||
| Expand Down Expand Up | @@ -219,7 +219,7 @@ def test_shaped_data(): | |
| y2 = np.arange( 10 ) | ||
| y2.shape = 10, 1 | ||
|
|
||
| fig = plt.figure(1);plt.clf() | ||
| plt.subplot( 411 ) | ||
| plt.plot( y1 ) | ||
| plt.subplot( 412 ) | ||
| Expand All | @@ -236,7 +236,7 @@ def test_shaped_data(): | |
|
|
||
| @image_comparison(baseline_images=['const_xy']) | ||
| def test_const_xy(): | ||
| fig = plt.figure(1);plt.clf() | ||
|
|
||
| plt.subplot( 311 ) | ||
| plt.plot( np.arange(10), np.ones( (10,) ) ) | ||
| Expand All | @@ -255,7 +255,7 @@ def test_const_xy(): | |
| def test_polar_wrap(): | ||
| D2R = np.pi / 180.0 | ||
|
|
||
| fig = plt.figure(1);plt.clf() | ||
|
|
||
| #NOTE: resolution=1 really should be the default | ||
| plt.subplot( 111, polar=True, resolution=1 ) | ||
| Expand All | @@ -265,7 +265,7 @@ def test_polar_wrap(): | |
|
|
||
| fig.savefig( 'polar_wrap_180' ) | ||
|
|
||
| fig = plt.figure(1);plt.clf() | ||
|
|
||
| #NOTE: resolution=1 really should be the default | ||
| plt.subplot( 111, polar=True, resolution=1 ) | ||
| Expand All | @@ -292,7 +292,7 @@ def test_polar_units(): | |
| y1 = [ 1.0, 2.0, 3.0, 4.0] | ||
| y2 = [ 4.0, 3.0, 2.0, 1.0 ] | ||
|
|
||
| fig = plt.figure(1);plt.clf() | ||
|
|
||
| plt.polar( x2, y1, color = "blue" ) | ||
|
|
||
| Expand All | @@ -312,7 +312,7 @@ def test_polar_rmin(): | |
| r = np.arange(0, 3.0, 0.01) | ||
| theta = 2*np.pi*r | ||
|
|
||
| fig = plt.figure(1);plt.clf() | ||
| ax = fig.add_axes([0.1, 0.1, 0.8, 0.8], polar=True) | ||
| ax.plot(theta, r) | ||
| ax.set_rmax(2.0) | ||
| Expand All | @@ -332,7 +332,7 @@ def test_axvspan_epoch(): | |
|
|
||
| dt = units.Duration( "ET", units.day.convert( "sec" ) ) | ||
|
|
||
| fig = plt.figure(1);plt.clf() | ||
|
|
||
| plt.axvspan( t0, tf, facecolor="blue", alpha=0.25 ) | ||
|
|
||
| Expand All | @@ -353,7 +353,7 @@ def test_axhspan_epoch(): | |
|
|
||
| dt = units.Duration( "ET", units.day.convert( "sec" ) ) | ||
|
|
||
| fig = plt.figure(1);plt.clf() | ||
|
|
||
| plt.axhspan( t0, tf, facecolor="blue", alpha=0.25 ) | ||
|
|
||
| Expand All | @@ -366,7 +366,7 @@ def test_axhspan_epoch(): | |
| @image_comparison(baseline_images=['hexbin_extent']) | ||
| def test_hexbin_extent(): | ||
| # this test exposes sf bug 2856228 | ||
| fig = plt.figure(1);plt.clf() | ||
|
|
||
| ax = fig.add_subplot(111) | ||
| data = np.arange(2000.)/2000. | ||
| Expand All | @@ -381,7 +381,7 @@ def test_nonfinite_limits(): | |
| x = np.arange(0., np.e, 0.01) | ||
| y = np.log(x) | ||
| x[len(x)/2] = np.nan | ||
| fig = plt.figure(1);plt.clf() | ||
| ax = fig.add_subplot(111) | ||
| ax.plot(x, y) | ||
| fig.savefig('nonfinite_limits') | ||
| Expand All | @@ -396,7 +396,7 @@ def test_imshow(): | |
| r = np.sqrt(x**2+y**2-x*y) | ||
|
|
||
| #Create a contour plot at N/4 and extract both the clip path and transform | ||
| fig = plt.figure(1);plt.clf() | ||
| ax = fig.add_subplot(111) | ||
|
|
||
| ax.imshow(r) | ||
| Expand All | @@ -414,7 +414,7 @@ def test_imshow_clip(): | |
| r = np.sqrt(x**2+y**2-x*y) | ||
|
|
||
| #Create a contour plot at N/4 and extract both the clip path and transform | ||
| fig = plt.figure(1);plt.clf() | ||
| ax = fig.add_subplot(111) | ||
|
|
||
| c = ax.contour(r,[N/4]) | ||
| Expand All | @@ -435,7 +435,7 @@ def test_polycollection_joinstyle(): | |
|
|
||
| from matplotlib import collections as mcoll | ||
|
|
||
| fig = plt.figure(1);plt.clf() | ||
| ax = fig.add_subplot(111) | ||
| verts = np.array([[1,1], [1,2], [2,2], [2,1]]) | ||
| c = mcoll.PolyCollection([verts], linewidths = 40) | ||
| Expand All | @@ -453,7 +453,7 @@ def test_fill_between_interpolate(): | |
| y1 = np.sin(2*np.pi*x) | ||
| y2 = 1.2*np.sin(4*np.pi*x) | ||
|
|
||
| fig = plt.figure(1);plt.clf() | ||
| ax = fig.add_subplot(211) | ||
| ax.plot(x, y1, x, y2, color='black') | ||
| ax.fill_between(x, y1, y2, where=y2>=y1, facecolor='green', interpolate=True) | ||
| Expand All | @@ -473,7 +473,7 @@ def test_symlog(): | |
| x = np.array([0,1,2,4,6,9,12,24]) | ||
| y = np.array([1000000, 500000, 100000, 100, 5, 0, 0, 0]) | ||
|
|
||
| fig = plt.figure(1);plt.clf() | ||
| ax = fig.add_subplot(111) | ||
| ax.plot(x, y) | ||
| ax.set_yscale('symlog') | ||
| Expand All | @@ -497,7 +497,7 @@ def test_pcolormesh(): | |
| # The color array can include masked values: | ||
| Zm = ma.masked_where(np.fabs(Qz) < 0.5*np.amax(Qz), Z) | ||
|
|
||
| fig = plt.figure(1);plt.clf() | ||
| ax = fig.add_subplot(121) | ||
| ax.pcolormesh(Qx,Qz,Z, lw=0.5, edgecolors='k') | ||
| ax.set_title('lw=0.5') | ||
| Expand All | @@ -515,10 +515,37 @@ def test_pcolormesh(): | |
|
|
||
| @image_comparison(baseline_images=['canonical']) | ||
| def test_canonical(): | ||
| plt.close('all') | ||
| fig, ax = plt.subplots(num=1) | ||
| ax.plot([1,2,3]) | ||
| fig.savefig('canonical') | ||
|
|
||
| def test_scrub(): | ||
| from matplotlib import rcParams | ||
| fig = plt.figure(1); fig.clf() | ||
| s = fig.subplotpars | ||
| fig.subplots_adjust(left=.3, bottom=.1, right=.4, | ||
| top=.15,hspace=.9,wspace=.26) | ||
| # with scrub=False, subplotpars should not be reset to the rcParam defaults | ||
| fig.clf(scrub=False) | ||
| assert s.left == .3 | ||
| assert s.bottom == .1 | ||
| assert s.right == .4 | ||
| assert s.top == .15 | ||
| assert s.hspace == .9 | ||
| assert s.wspace == .26 | ||
|
|
||
| fig.subplots_adjust(left=.3, bottom=.1, right=.4, | ||
| top=.15,hspace=.9,wspace=.26) | ||
| # with scrub=True, subplotpars should be reset to the rcParam defaults | ||
| fig.clf(scrub=True) | ||
| assert s.left == rcParams['figure.subplot.left'] | ||
| assert s.bottom == rcParams['figure.subplot.bottom'] | ||
| assert s.right == rcParams['figure.subplot.right'] | ||
| assert s.top == rcParams['figure.subplot.top'] | ||
| assert s.hspace == rcParams['figure.subplot.hspace'] | ||
| assert s.wspace == rcParams['figure.subplot.wspace'] | ||
|
|
||
| if __name__=='__main__': | ||
| import nose | ||
| nose.runmodule(argv=['-s','--with-doctest'], exit=False) | ||
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
Loading
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityIMHO the rc parameter is overkill. We could default to False and let users who like the scrubbing clear define an alias def scrub(): clf(scrub=True), or we could even define it for them.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
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.
Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Qualitythe reason I went with the kwarg is because I think users would have a reasonable expectation to have the clear figure command to clear the figure completely, as opposed to having to call a separate command for doing that. But given that others might want to keep the old behavior, we can leave as a user configurable option. The new default behavior (figure.autoscrub = True) allows for the plt.clf() commands to stay as is in the rest of the pull request (though I realize you don't like the code duplication).
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.