Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Custom error message for draw_path. issues : #8131 (bad error message from pyplot.plot) #8176
Conversation
|
Hi @herilalaina 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. |
|
Hi @NelleV, no problem. I will make this change as soon as I can :) |
tacaswell
added this to the
2.1 (next point release)
milestone
Mar 8, 2017
| + try: | ||
| + self._renderer.draw_path(gc, p, transform, rgbFace) | ||
| + except OverflowError as e: | ||
| + e.args = ("Exceeded cell block limit (set 'agg.path.chunksize' rcparam)",) |
herilalaina
Mar 9, 2017
Contributor
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
QuLogic
Mar 12, 2017
Member
I mean, is changing e.args a good thing to do, vs raising a new exception entirely.
herilalaina
Mar 13, 2017
Contributor
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 limitDuring 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.
QuLogic
Mar 25, 2017
Member
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?
herilalaina
Mar 27, 2017
Contributor
Hi @QuLogic , checks failed on coverage. I'm new to code coverage. Do I need to implement test (for overflow error) to fix that ?
| else: | ||
| - self._renderer.draw_path(gc, path, transform, rgbFace) | ||
| + try: | ||
| + self._renderer.draw_path(gc, p, transform, rgbFace) |
| npts = path.vertices.shape[0] | ||
| + | ||
| if (nmax > 100 and npts > nmax and path.should_simplify and |
|
|
herilalaina
added some commits
Apr 4, 2017
|
|
herilalaina |
3f796d9
|
|||
|
|
herilalaina |
Some checks were not successful |
75be34d
|
| + rcParams['agg.path.chunksize'] = 8000000 | ||
| + with pytest.raises(OverflowError): | ||
| + ax.plot(x, np.sin(x)) | ||
| + plt.show() |
herilalaina
Apr 5, 2017
Contributor
Hi @tacaswell , it doesn't raise OverflowError. Do you have any idea how to handle it?
herilalaina
Apr 10, 2017
Contributor
It never raises overflow even if I increase chunk size. The test just crashed
| +def test_chunksize(): | ||
| + x = range(9000000) | ||
| + fig = plt.figure() | ||
| + ax = fig.add_subplot(1, 1, 1) |
| + ax = fig.add_subplot(1, 1, 1) | ||
| + | ||
| + # Test with small chunksize | ||
| + tmp = rcParams['agg.path.chunksize'] |
QuLogic
Apr 12, 2017
Member
You don't need to worry about this; the cleanup on the test will reset all rcParams.
|
I think the test is not doing anything because of the |
|
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. |
|
After some testing, it looks like you need roughly |
|
Thank you for your great feedback ! I will fix them. |
herilalaina
added some commits
Apr 14, 2017
|
|
herilalaina |
Some checks were not successful |
f00156f
|
||
|
|
herilalaina |
665c640
|
herilalaina commentedMar 1, 2017
Addressing the issue #8131, this PR adds reference to the agg.path.chunksize params in the overflow error message.