Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Fix for _axes.scatter() array index out of bound error#12673
New issue
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Seems to be the same as#12672 |
#12672 at the time of writing this post still uses |
Looks like this also needs a test for the use case that was causing the original issue? |
Sure, I can put one in later. Thought that since we used existing helper we didn’t need one, but probably best to add that test case. …Sent from my iPhone On 31 Oct 2018, at 01:10, Jody Klymak ***@***.***> wrote: Looks like this also needs a test for the use case that was causing the original issue? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread. |
You want a test that tests the scatter use cases so no one inadvertently breaks this functionaility in the future by modifying your check. The API is quite flexible (some might say too flexible) and its hard to anticipate every usecase when making changes, so we hope the tests catch them. |
How does this look? 2 new test cases added, fixed a an easy formatting line on the way. Borrowed the empty test case from@timhoffm. The other test case is for the non-zero indexed |
Uh oh!
There was an error while loading.Please reload this page.
Sure, let me look into it tonight. …Sent from my iPhone On 1 Nov 2018, at 05:23, Tim Hoffmann ***@***.***> wrote:@timhoffm commented on this pull request. In lib/matplotlib/tests/test_axes.py: > @@ -5856,3 +5856,17 @@ def test_gettightbbox_ignoreNaN(): t = ax.text(np.NaN, 1, 'Boo') renderer = fig.canvas.get_renderer() np.testing.assert_allclose(ax.get_tightbbox(renderer).width, 532.444444) + + +def test_scatter_series_non_zero_index(pd): + # create non-zero index + ids = range(10, 18) + x = pd.Series(np.random.uniform(size=8), index=ids) + y = pd.Series(np.random.uniform(size=8), index=ids) + c = pd.Series([1, 1, 1, 1, 1, 0, 0, 0], index=ids) + plt.scatter(x, y, c) + + +def test_scatter_empty_data(): + plt.scatter([], []) Can please add a comment # just making sure this does not raise an exception as I did in my PR? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread. |
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.
Added comment in test as suggested.
Thanks for taking this on,@esvhd |
Owee, I'm MrMeeseeks, Look at me. There seem to be a conflict, please backport manually. Here are approximate instructions:
And apply the correct labels and milestones. Congratulation you did some good work ! Hopefully your backport PR will be tested by the continuous integration and merged soon! If these instruction are inaccurate, feel free tosuggest an improvement. |
Thanks@phobson. Is there anything I need to do about the backport comment posted by the bot? Happy to take this over the finish line. |
Fix for _axes.scatter() array index out of bound error
Trying to backport this currently fails the tests for 3.5. See#12761. |
…of-pr-12673-on-v3.0.xBackport PR#12673 to v3.0.x
PR Summary
This is to issue#12641, fixing the index out of bound error for accessing first element of an Iterable. Solution uses
cbook.safe_first_element().Added test case for
cbook.safe_first_element()forpandas.Seriesobject.