| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
|
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. |
Sorry, something went wrong.
|
Hi @NelleV, no problem. I will make this change as soon as I can :) |
Sorry, something went wrong.
There was a problem hiding this comment.
Typo needs to be fixed; tests are broken.
Sorry, something went wrong.
| else: | ||
| self._renderer.draw_path(gc, path, transform, rgbFace) | ||
| try: | ||
| self._renderer.draw_path(gc, p, transform, rgbFace) |
There was a problem hiding this comment.
Should be path, not p.
Sorry, something went wrong.
There was a problem hiding this comment.
Sorry about that. thank you
Sorry, something went wrong.
| try: | ||
| self._renderer.draw_path(gc, p, transform, rgbFace) | ||
| except OverflowError as e: | ||
| e.args = ("Exceeded cell block limit (set 'agg.path.chunksize' rcparam)",) |
There was a problem hiding this comment.
I'm not sure about this; should we raise a new exception?
Sorry, something went wrong.
There was a problem hiding this comment.
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
Sorry, something went wrong.
There was a problem hiding this comment.
I mean, is changing e.args a good thing to do, vs raising a new exception entirely.
Sorry, something went wrong.
There was a problem hiding this comment.
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.
Sorry, something went wrong.
There was a problem hiding this comment.
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?
Sorry, something went wrong.
There was a problem hiding this comment.
Hi @QuLogic , checks failed on coverage. I'm new to code coverage. Do I need to implement test (for overflow error) to fix that ?
Sorry, something went wrong.
| nmax = rcParams['agg.path.chunksize'] # here at least for testing | ||
| npts = path.vertices.shape[0] | ||
|
|
||
| if (nmax > 100 and npts > nmax and path.should_simplify and |
There was a problem hiding this comment.
This half of the if/else block is untested. Think you could add a test?
Sorry, something went wrong.
There was a problem hiding this comment.
hi @phobson , thank you. I will do it
Sorry, something went wrong.
|
👍 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? |
Sorry, something went wrong.
| rcParams['agg.path.chunksize'] = 8000000 | ||
| with pytest.raises(OverflowError): | ||
| ax.plot(x, np.sin(x)) | ||
| plt.show() |
There was a problem hiding this comment.
Hi @tacaswell , it doesn't raise OverflowError. Do you have any idea how to handle it?
Sorry, something went wrong.
There was a problem hiding this comment.
Is that chunk size big enough to cause a overflow?
Sorry, something went wrong.
There was a problem hiding this comment.
It never raises overflow even if I increase chunk size. The test just crashed
Sorry, something went wrong.
| ax = fig.add_subplot(1, 1, 1) | ||
|
|
||
| # Test with small chunksize | ||
| tmp = rcParams['agg.path.chunksize'] |
There was a problem hiding this comment.
You don't need to worry about this; the cleanup on the test will reset all rcParams.
Sorry, something went wrong.
| def test_chunksize(): | ||
| x = range(9000000) | ||
| fig = plt.figure() | ||
| ax = fig.add_subplot(1, 1, 1) |
There was a problem hiding this comment.
fig, ax = plt.subplots()
Sorry, something went wrong.
|
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... |
Sorry, something went wrong.
|
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. |
Sorry, something went wrong.
|
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. |
Sorry, something went wrong.
|
Thank you for your great feedback ! I will fix them. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Addressing the issue #8131, this PR adds reference to the agg.path.chunksize params in the overflow error message.