Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Prepare for ragged array warnings in NumPy 1.19#17289

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

Merged
jklymak merged 7 commits intomatplotlib:masterfromQuLogic:np119
May 22, 2020

Conversation

QuLogic
Copy link
Member

@QuLogicQuLogic commentedMay 1, 2020•
edited by tacaswell
Loading

PR Summary

NumPy 1.19 will issue a deprecation warning about ragged arraysnumpy/numpy#15119, which causes our tests to fail as we fail on warning. This fixes the easy things, but there are 4 more classes of failures:

  • cbook._reshape_2D's test checking that a ragged array is turned into an object array.FixesBUG: VisibleDeprecationWarning in boxplot #16353
  • hist casting raggedx input to a 2d array (most of the failures are this one)
    • this should be fixed by the first one
  • test_scatter_marker passes an array of color-likes, which goes through_combine_masks which tries to cast as an array
    • We could tell_combine_masks to use object array, or pass a flag to allow it? I'm not sure if it should always try to use an object array.
  • test_bad_plot_args passes some ridiculous values, and I don't know if they should be checked earlier or just allow it to fail when NumPy turns this into an error

I haven't just changed these to explicitly ask for object arrays because I haven't yet figured out what the implications are in NumPy and why they started warning.

PR Checklist

  • Has Pytest style unit tests
  • Code isFlake 8 compliant
  • [N/A] New features are documented, with examples if plot related
  • [N/A] Documentation is sphinx and numpydoc compliant
  • [N/A] Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • [N/A] Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

@tacaswell
Copy link
Member

Do you want to get these is as-is and address the rest later or hold these to fix them all in one shot?

@QuLogic
Copy link
MemberAuthor

Ah, telling_reshape_2D to useobject dtype breaksviolinplot instead...

@QuLogicQuLogic marked this pull request as ready for reviewMay 6, 2020 04:58
@QuLogic
Copy link
MemberAuthor

This is mostly done, except fortest_bad_plot_args, and I'm not sure what to do there. That test does:

plt.plot((np.arange(5).reshape((1,-1)),np.arange(5).reshape(-1,1)))

which is of course, non-sensical. It currently checks that it raises aValueError. Unlike the other inputs, which raise in_plot_args, this one actually works fine, and raises somewhere deep inLine2D basically trying to draw. Ithink this case should have been caught in_plot_args, but I'm not sure how to check it safely.

@QuLogic
Copy link
MemberAuthor

Not sure if that's the best last commit, but that should fix all the warnings.

@tacaswell
Copy link
Member

I pushed a commit to fixtest_violinplot_pandas_series[png] that was failing for me locally.

@tacaswelltacaswell added the Release criticalFor bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. labelMay 16, 2020
@tacaswell
Copy link
Member

Do we want to backport this to 3.2.x?

x = np.asanyarray(x)
try:
x = np.asanyarray(x)
except np.VisibleDeprecationWarning:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

So, what happens after the deprecation period?

Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I assume it'll be aValueError, but it wasn't clear in the NEP.

QuLogicand others added7 commitsMay 21, 2020 23:08
The `segments` parameter is a list of lines (a line being a 2D array orlist of 2-tuples), but every line is not required to be the same length.This would cause NumPy to produce an object array, but it will start towarn about ragged arrays in 1.19.An array isn't really needed as `Line3DCollection._segments3d` is onlyiterated over.
These inputs may be ragged, but we only just need to know the length ofthe items. There's no need to make an array to check that.
This is a raising a deprecation warning in NumPy 1.19, may go away sometime later, and is not strictly necessary for the implementation.
This causes a NumPy deprecation warning if the list is ragged.
This function accepts almost anything list-like, so we really do wantconversion to object arrays if the input is ragged. For example, thismight occur for `scatter()` if passed different types of `colors`, likein the `test_scatter_marker` test.
@jklymakjklymak merged commit5ab1352 intomatplotlib:masterMay 22, 2020
@jklymak
Copy link
Member

Oh, hmmm, didn't notice that had a bunch of commits. Sorry if that was meant to be squashed...

@QuLogicQuLogic deleted the np119 branchMay 22, 2020 05:07
tacaswell pushed a commit to tacaswell/matplotlib that referenced this pull requestJun 10, 2020
Prepare for ragged array warnings in NumPy 1.19Conflicts:lib/matplotlib/axes/_axes.py         - implicitly backported changes to wording in error messages
@tacaswell
Copy link
Member

Do we want to backport this to 3.2.2? I tried cherry-picking and there is 12 failures on 3.2.x with this backported and numpy master.

I guess our choices are :

  1. backport nothing
  2. backport, but don't clean up the other failures (most of them look like they are in scatter's input munging)
  3. backport and clean up all the other warnings.

I'm between 1 and 3. Doing it half way seems like a poor life choice and I am not sure that it is worth the effort for 3.

@QuLogic
Copy link
MemberAuthor

At least for Fedora, this would not be a problem as they would not show up at the same time. We can possibly just say they aren't supported on other systems (mainly conda-forge, I'd assume).

I tried the backport and there are about 11 extra failures. I believe that most of them are fixed by#15834, as that removes thenp.array call, but I have not tested it out, as I'm not sure we want to actually backport that.

@tacaswell
Copy link
Member

Given that this caused a regression I am glad that we opted not to backport.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@timhoffmtimhoffmtimhoffm left review comments

@tacaswelltacaswelltacaswell approved these changes

@jklymakjklymakjklymak approved these changes

Assignees
No one assigned
Labels
MaintenanceRelease criticalFor bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions.
Projects
None yet
Milestone
v3.3.0
Development

Successfully merging this pull request may close these issues.

BUG: VisibleDeprecationWarning in boxplot
4 participants
@QuLogic@tacaswell@jklymak@timhoffm
[8]ページ先頭
©2009-2025 Movatter.jp