Custom error message for draw_path. issues : #8131 (bad error message from pyplot.plot) #8176

herilalaina · 2017-03-01T15:33:30Z

Addressing the issue #8131, this PR adds reference to the agg.path.chunksize params in the overflow error message.

NelleV · 2017-03-08T06:25:21Z

Hi @herilalaina
Sorry it took so long for us to review the pull request and thanks for the patch!

I'd rather have the try except surround only the part of the code that can raise an overflow error for readabiblity. It means having two try except, but I think we can live with that.
Do you mind making this small change?

herilalaina · 2017-03-08T10:18:20Z

Hi @NelleV, no problem. I will make this change as soon as I can :)

QuLogic

Typo needs to be fixed; tests are broken.

QuLogic · 2017-03-09T07:17:24Z

lib/matplotlib/backends/backend_agg.py

        else:
-            self._renderer.draw_path(gc, path, transform, rgbFace)
+            try:
+                self._renderer.draw_path(gc, p, transform, rgbFace)


Should be path, not p.

Sorry about that. thank you

QuLogic · 2017-03-09T07:19:35Z

lib/matplotlib/backends/backend_agg.py

+                try:
+                    self._renderer.draw_path(gc, p, transform, rgbFace)
+                except OverflowError as e:
+                    e.args = ("Exceeded cell block limit (set 'agg.path.chunksize' rcparam)",)


I'm not sure about this; should we raise a new exception?

I am not sure I understand your point. In fact, the same overflow error occurs if we set agg.path.chunksize with a big number. Here, I just change the overflow error message

I mean, is changing e.args a good thing to do, vs raising a new exception entirely.

in fact, if we raise new overflow error. We get something like this

Exception in Tkinter callback
Traceback (most recent call last):
File "/home/herilalaina/Documents/contribution/matplotlib_contrib/matplotlib/lib/matplotlib/backends/backend_agg.py", line 171, in draw_path
self._renderer.draw_path(gc, path, transform, rgbFace)
OverflowError: In draw_path: Exceeded cell block limit

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
File "/home/herilalaina/Documents/contribution/matplotlib_contrib/matplotlib_env/lib/python3.5/tkinter/init.py", line 1562, in call
return self.func(*args)
File "/home/herilalaina/Documents/contribution/matplotlib_contrib/matplotlib/lib/matplotlib/backends/backend_tkagg.py", line 284, in resize
self.show()
File "/home/herilalaina/Documents/contribution/matplotlib_contrib/matplotlib/lib/matplotlib/backends/backend_tkagg.py", line 355, in draw
FigureCanvasAgg.draw(self)
File "/home/herilalaina/Documents/contribution/matplotlib_contrib/matplotlib/lib/matplotlib/backends/backend_agg.py", line 475, in draw
self.figure.draw(self.renderer)
File "/home/herilalaina/Documents/contribution/matplotlib_contrib/matplotlib/lib/matplotlib/artist.py", line 68, in draw_wrapper
return draw(artist, renderer, *args, **kwargs)
File "/home/herilalaina/Documents/contribution/matplotlib_contrib/matplotlib/lib/matplotlib/figure.py", line 1240, in draw
renderer, self, artists, self.suppressComposite)
File "/home/herilalaina/Documents/contribution/matplotlib_contrib/matplotlib/lib/matplotlib/image.py", line 139, in _draw_list_compositing_images
a.draw(renderer)
File "/home/herilalaina/Documents/contribution/matplotlib_contrib/matplotlib/lib/matplotlib/artist.py", line 68, in draw_wrapper
return draw(artist, renderer, *args, **kwargs)
File "/home/herilalaina/Documents/contribution/matplotlib_contrib/matplotlib/lib/matplotlib/axes/_base.py", line 2386, in draw
mimage._draw_list_compositing_images(renderer, self, artists)
File "/home/herilalaina/Documents/contribution/matplotlib_contrib/matplotlib/lib/matplotlib/image.py", line 139, in _draw_list_compositing_images
a.draw(renderer)
File "/home/herilalaina/Documents/contribution/matplotlib_contrib/matplotlib/lib/matplotlib/artist.py", line 68, in draw_wrapper
return draw(artist, renderer, *args, **kwargs)
File "/home/herilalaina/Documents/contribution/matplotlib_contrib/matplotlib/lib/matplotlib/lines.py", line 800, in draw
line_func(renderer, gc, tpath, affine.frozen())
File "/home/herilalaina/Documents/contribution/matplotlib_contrib/matplotlib/lib/matplotlib/lines.py", line 1248, in _draw_solid
renderer.draw_path(gc, path, trans)
File "/home/herilalaina/Documents/contribution/matplotlib_contrib/matplotlib/lib/matplotlib/backends/backend_agg.py", line 173, in draw_path
raise OverflowError("Exceeded cell block limit (set 'agg.path.chunksize' rcparam)")
OverflowError: Exceeded cell block limit (set 'agg.path.chunksize' rcparam)

I will make the change if It's okay.

Obviously, it does appear to work, but I don't have an answer whether it is a good or idiomatic thing to be doing or not.

Opinions, @tacaswell?

Hi @QuLogic , checks failed on coverage. I'm new to code coverage. Do I need to implement test (for overflow error) to fix that ?

phobson · 2017-03-29T00:16:41Z

lib/matplotlib/backends/backend_agg.py

        npts = path.vertices.shape[0]
+
        if (nmax > 100 and npts > nmax and path.should_simplify and


This half of the if/else block is untested. Think you could add a test?

hi @phobson , thank you. I will do it

tacaswell · 2017-04-04T03:28:24Z

👍 on this. To test the missed branch I assume you need to user a very long line with a chunk size that is too big?

herilalaina · 2017-04-05T09:04:48Z

lib/matplotlib/tests/test_agg.py

+    rcParams['agg.path.chunksize'] = 8000000
+    with pytest.raises(OverflowError):
+        ax.plot(x, np.sin(x))
+        plt.show()


Hi @tacaswell , it doesn't raise OverflowError. Do you have any idea how to handle it?

Is that chunk size big enough to cause a overflow?

It never raises overflow even if I increase chunk size. The test just crashed

QuLogic · 2017-04-12T09:27:18Z

lib/matplotlib/tests/test_agg.py

+    ax = fig.add_subplot(1, 1, 1)
+
+    # Test with small chunksize
+    tmp = rcParams['agg.path.chunksize']


You don't need to worry about this; the cleanup on the test will reset all rcParams.

QuLogic · 2017-04-12T09:27:30Z

lib/matplotlib/tests/test_agg.py

+def test_chunksize():
+    x = range(9000000)
+    fig = plt.figure()
+    ax = fig.add_subplot(1, 1, 1)


fig, ax = plt.subplots()

QuLogic · 2017-04-12T09:41:06Z

I think the test is not doing anything because of the plt.show; replacing it with fig.canvas.draw() at least triggers something, though it still doesn't seem to cause an exception...

QuLogic · 2017-04-12T09:46:58Z

Actually, if all you want to do is test that side of the branch, you only need a path longer than the chunksize (which must be greater than 100), so set the chunksize to 105 and make a path that's 110 long. 9000000 is much too long, I think.

If you want to trigger the exception, that might be a little more difficult and I'm not sure how to do that.

QuLogic · 2017-04-13T03:30:24Z

After some testing, it looks like you need roughly range(200000) to trigger the exception with chunksize<=100. That means that you'd need approximately range(20000000) to trigger an exception with chunksize>100. I think this is basically impossible so I don't think we should try to trigger the exception in the latter case.

herilalaina · 2017-04-13T07:48:57Z

Thank you for your great feedback ! I will fix them.

tacaswell added this to the 2.1 (next point release) milestone Mar 8, 2017

QuLogic requested changes Mar 9, 2017

View reviewed changes

Add exception handling

f7d1250

herilalaina force-pushed the error-msg-plot branch 3 times, most recently from eec2383 to 50dbb8f Compare March 27, 2017 08:55

phobson reviewed Mar 29, 2017

View reviewed changes

herilalaina force-pushed the error-msg-plot branch 15 times, most recently from 55abee1 to df10f02 Compare April 4, 2017 16:12

Add chunksize test for backend agg

3f796d9

herilalaina force-pushed the error-msg-plot branch from df10f02 to 3f796d9 Compare April 4, 2017 16:23

Delete test on overflowError

75be34d

herilalaina commented Apr 5, 2017

View reviewed changes

QuLogic reviewed Apr 12, 2017

View reviewed changes

Fix test_chunksize on test_agg

f00156f

herilalaina force-pushed the error-msg-plot branch from d82cae8 to f00156f Compare April 14, 2017 09:10

Change chunksize

665c640

herilalaina force-pushed the error-msg-plot branch from 87f3f66 to 665c640 Compare April 26, 2017 19:51

QuLogic approved these changes Apr 29, 2017

View reviewed changes

QuLogic mentioned this pull request Apr 30, 2017

API: remove agg path chunking logic #4464

Closed

phobson merged commit 1ef4d39 into matplotlib:master Apr 30, 2017

herilalaina deleted the error-msg-plot branch June 8, 2017 14:06

anntzer mentioned this pull request Sep 11, 2017

OverflowError: In draw_path: Exceeded cell block limit #5907

Closed

anntzer mentioned this pull request Oct 20, 2017

bad error message from pyplot.plot #8131

Closed

		npts = path.vertices.shape[0]

		if (nmax > 100 and npts > nmax and path.should_simplify and

Uh oh!

Custom error message for draw_path. issues : #8131 (bad error message from pyplot.plot) #8176

Custom error message for draw_path. issues : #8131 (bad error message from pyplot.plot) #8176

Uh oh!

Conversation

herilalaina commented Mar 1, 2017

Uh oh!

NelleV commented Mar 8, 2017

Uh oh!

herilalaina commented Mar 8, 2017

Uh oh!

QuLogic left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tacaswell commented Apr 4, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

QuLogic commented Apr 12, 2017

Uh oh!

QuLogic commented Apr 12, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

QuLogic commented Apr 13, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

herilalaina commented Apr 13, 2017

Uh oh!

Uh oh!

QuLogic commented Apr 12, 2017 •

edited

Loading

QuLogic commented Apr 13, 2017 •

edited

Loading