Fix various issues from SonarQube #23494
Conversation
| @@ -309,10 +309,9 @@ def test_add_collection(): | |||
| # GitHub issue #1490, pull #1497. | |||
| plt.figure() | |||
| ax = plt.axes() | |||
| coll = ax.scatter([0, 1], [0, 1]) | |||
| ax.add_collection(coll) | |||
There was a problem hiding this comment.
This is funny enough looking I'm worried this was intentional to test double adding artists or some such.
There was a problem hiding this comment.
The comment about what the test does and the name of the test seem to be a bit in disagreement. Not sure if it's intentional, but by the comment, it seems like it shouldn't be there...
There was a problem hiding this comment.
Actually, looking at the original PR and issue, it's definitely about empty collections, and the duplicate add_collection seems to be an error.
| @@ -34,11 +34,11 @@ | |||
| def onpick(event): | |||
|
|
|||
| if event.artist != line: | |||
| return True | |||
There was a problem hiding this comment.
This changes the return type and bool value of the return. Do we not actually use this return?
There was a problem hiding this comment.
No event handlers use the return value; CallbackRegistry does not do anything with it.
| if animation.ax.get_renderer_cache(): | ||
| animation.draw() | ||
| start_anim.timer.add_callback(local_draw) | ||
| start_anim.timer.add_callback(animation.draw) |
There was a problem hiding this comment.
This is changing the semantics to call draw() unconditionally. I don't claim to understand why the original code did not draw if there is a renderer_cache, but we should now why it's been there and can now leave it out (@tacaswell you wrote this originally).
Semi-OT: This removes the only usage of Axes.get_renderer_cache, which is undocumented by the way. Do we still need to expose this? If yes, this needs documentation, if no we should deprecate it.
There was a problem hiding this comment.
I suspect that this was a hedge to make sure we were actually drawn once before running because the draw method on Animation requires use Axes.draw_artist which in turn requires the Axes to have been drawn once to get a cached render.
However, we have made a lot of progress in pushing the caching of renderers into exactly 1 place and only access them via a function that will fabricate one if needed so I suspect we may no longer need this.
There was a problem hiding this comment.
See above: #23494 (comment)
There was a problem hiding this comment.
See #23202 for removal of the Axes.get_renderer_cache
Also, document `iqr` return value.
This matches the 2D case, and fixes the docstring.
5d71d14 to
a9ec6aa
Compare
PR Summary
Mostly removing dead variables and returns, ensuring consistency between subclass method signatures, and a bit of cleanup.
PR Checklist
Tests and Styling
pytestpasses).flake8-docstringsand runflake8 --docstring-convention=all).Documentation
doc/users/next_whats_new/(follow instructions in README.rst there).doc/api/next_api_changes/(follow instructions in README.rst there).