8000 Handle matplotlib 2.2 pre-release deprecations by soupault · Pull Request #2977 · scikit-image/scikit-image · GitHub
[go: up one dir, main page]

Skip to content

Handle matplotlib 2.2 pre-release deprecations #2977

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 7, 2018

Conversation

soupault
Copy link
Member
@soupault soupault commented Mar 5, 2018

Description

I checked several gallery examples modified by this PR in my Python environment (matplotlib 2.1.0) and haven't found any regressions.

References

Closes #2965.

MPL PRs:

For reviewers

(Don't remove the checklist below.)

  • Check that the PR title is short, concise, and will make sense 1 year
    later.
  • Check that new functions are imported in corresponding __init__.py.
  • Check that new features, API changes, and deprecations are mentioned in
    doc/release/release_dev.rst.

@soupault soupault added 🔧 type: Maintenance Refactoring and maintenance of internals 🧐 Needs review labels Mar 5, 2018
@pep8speaks
Copy link

Hello @soupault! Thanks for submitting the PR.

Line 62:80: E501 line too long (82 > 79 characters)

@soupault
Copy link
Member Author
soupault commented Mar 5, 2018

@efiring @tacaswell Hi! Do you have any ideas on what kind of recursion happened in this build - https://travis-ci.org/soupault/scikit-image/jobs/349250897 ?

@efiring
Copy link
efiring commented Mar 5, 2018

Given that the failure occurred on only one of the test configurations I suspect it is a Travis fluke.

@stefanv
Copy link
Member
stefanv commented Mar 5, 2018

@efiring That seems unlikely, given that the traceback is matplotlib specific:

https://travis-ci.org/scikit-image/scikit-image/jobs/349286622#L5741

This is the file in question: https://github.com/scikit-image/scikit-image/blob/master/doc/examples/color_exposure/plot_ihc_color_separation.py

And the configuration for this builder has the Qt backend enabled. There seems to be a recursion when draw is called.

@soupault
Copy link
Member Author
soupault commented Mar 6, 2018

Travis cache cleanup doesn't seem to help.

@efiring
Copy link
efiring commented Mar 6, 2018

matplotlib/matplotlib#10690 is addressing the only known present recursion problem.

@efiring
Copy link
efiring commented Mar 6, 2018

The plotting in that function looks pretty generic. I take it the problem is repeatable, but only with that one build configuration, correct? What is the implication of enabling the Qt backend in the test? I don't understand what that is doing, since presumably there is no window available. If you run the test code normally, on a computer with a display and with backend_qt5agg, do you still have a problem?

@stefanv stefanv merged commit 2c0f0b2 into scikit-image:master Mar 7, 2018
@stefanv
Copy link
Member
stefanv commented Mar 7, 2018

I'm doing some optimistic merging here, since matplotlib 2.2 is out.

@soupault soupault deleted the mpl22_fix branch March 7, 2018 10:56
@soupault
Copy link
Member Author
soupault commented Mar 7, 2018

If you run the test code normally, on a computer with a display and with backend_qt5agg, do you still have a problem?

I've run the example on Python 2.7 with matplotlib 2.2 locally, and haven't spotted any issues. Not sure if my environment 100% matches the one on Travis though.

hmaarrfk pushed a commit to hmaarrfk/scikit-image that referenced this pull request May 19, 2018
* Remove deprecated box-forced matplotlib parameter
* Use nipy_spectral colormap instead of deprecated spectral (mpl gh7416)
* Refactored plot_adapt_rgb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🧐 Needs review 🔧 type: Maintenance Refactoring and maintenance of internals
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants
0