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

Fixed categorical coloring of Contours in matplotlib #2259

Merged
merged 2 commits into from Feb 5, 2018

Conversation

Projects
None yet
2 participants
@philippjfr
Member

philippjfr commented Jan 14, 2018

As the title says, Contours/Polygons in matplotlib did not handle categorical values for the color_index.

@jlstevens

This comment has been minimized.

Member

jlstevens commented Jan 15, 2018

I can't tell if this PR is ready yet, but at a glance it looks good. Happy to merge once you tell me it is ready and the tests are green.

@philippjfr

This comment has been minimized.

Member

philippjfr commented Jan 15, 2018

Can't be ready since the tests aren't green right :-) Locally it seems to pass but Travis isn't happy yet. Will try to figure this out today.

@jlstevens

This comment has been minimized.

Member

jlstevens commented Jan 15, 2018

Can't be ready since the tests aren't green right...

Could have just been unlucky with transient failures you know...

@philippjfr

This comment has been minimized.

Member

philippjfr commented Jan 15, 2018

Could have just been unlucky with transient failures you know...

The unit test I added is failing.

@@ -213,7 +213,7 @@ def values(cls, dataset, dim, expanded=True, flat=True):
if np.isscalar(values):
if not expanded:
return np.array([values])
values = np.full(len(dataset), values)
values = np.full(len(dataset), values, dtype=np.array(values).dtype)

This comment has been minimized.

@philippjfr

philippjfr Feb 4, 2018

Member

This is the most bizarre bug I've come across in a long time. Before I added this dtype approach this line would error (but only on Travis), given this input:np.full(10, 'B'), saying that:

ValueError: could not convert string to float: 'B'

But internally np.full does exactly the same thing I've now done here to fix it:

def full(shape, fill_value, dtype=None, order='C'):
    if dtype is None:
        dtype = array(fill_value).dtype
    a = empty(shape, dtype, order)
    multiarray.copyto(a, fill_value, casting='unsafe')
    return a

Absolutely baffling, but it seems to work now.

@philippjfr

This comment has been minimized.

Member

philippjfr commented Feb 4, 2018

Ready now.

@jlstevens

This comment has been minimized.

Member

jlstevens commented Feb 5, 2018

Weird 'fix'!

Anyway, looks good. Merging.

@jlstevens jlstevens merged commit 263ae6a into master Feb 5, 2018

4 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.01%) to 80.943%
Details
s3-reference-data-cache Test data is cached.
Details

@philippjfr philippjfr added this to the 1.9.3 milestone Feb 11, 2018

@philippjfr philippjfr deleted the mpl_contours_categorical branch Feb 11, 2018

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