Skip to content

Fix colorbar test and color level determination for contour #8737

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 2 commits into from
Jun 12, 2017

Conversation

dstansby
Copy link
Member

@dstansby dstansby commented Jun 9, 2017

test_given_colors_levels_and_extends() is failing with numpy 1.13, and I think the failure exposes a wider problem with contour colours.

If plotting a non-filled contour, the number of colours needed is the same as the number of levels regardless of whether the colorbar is extended or not. This is why the indices i0 and i1 are now only modified if self.filled is True in lib/matplotlib/contour.py.

If plotting a filled contour, the number of colours depends on whether the colorbar is extended or not. This is dealt with by the logic that I've added a tab to in lib/matplotlib/contour.py

I have also changed the test, so that the colour of each filled level/contour level is the same across the images.

Finally, I think that there should probably be a check for the correct length of the colors kwarg that throws a sensible error message if it is wrong. Currently (starting with numpy 1.13) a cryptic index error will be raised (see https://travis-ci.org/matplotlib/matplotlib/jobs/241271218#L1767)

@tacaswell tacaswell requested a review from efiring June 9, 2017 20:03
@tacaswell tacaswell added this to the 2.1 (next point release) milestone Jun 9, 2017
@tacaswell
Copy link
Member

@efiring has to sign off on this before it gets merged.

@dstansby
Copy link
Member Author

dstansby commented Jun 9, 2017

(fixes #7334 if what I've done is correct)

Copy link
Member

@efiring efiring left a comment

Choose a reason for hiding this comment

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

Looks good, thank you.

@anntzer anntzer merged commit 166a144 into matplotlib:master Jun 12, 2017
@anntzer
Copy link
Contributor

anntzer commented Jun 12, 2017

thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants