Skip to content

Fix colorbar test and color level determination for contour - #8737

Merged
anntzer merged 2 commits into
matplotlib:masterfrom
dstansby:fix-tests
Jun 12, 2017
Merged

anntzer merged 2 commits into
matplotlib:masterfrom
dstansby:fix-tests

Conversation

@dstansby

@dstansby dstansby commented Jun 9, 2017

Copy link
Copy Markdown
Member

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
Copy Markdown
Member

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

@dstansby

dstansby commented Jun 9, 2017

Copy link
Copy Markdown
Member Author

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

@efiring efiring left a comment

Copy link
Copy Markdown
Member

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

anntzer commented Jun 12, 2017

Copy link
Copy Markdown
Contributor

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