Avoid comparing numpy array to strings in two places #7658

Merged
merged 3 commits into from Dec 22, 2016

Conversation

Projects
None yet
4 participants
Contributor

dstansby commented Dec 21, 2016

Fixes #5806

@dstansby dstansby Check if aspect is a string to avoid comparing numpy array to tuple o…
…f strings
a33be77
@efiring

Apart from the suggested change in implementation, this is good.

lib/matplotlib/axes/_base.py
@@ -1282,7 +1282,7 @@ def set_aspect(self, aspect, adjustable=None, anchor=None):
etc.
===== =====================
"""
- if aspect in ('equal', 'auto'):
+ if isinstance(aspect, str) and aspect in ('equal', 'auto'):
@efiring

efiring Dec 21, 2016

Owner

It might be better to use cbook.is_string_like instead of is_instance. Or at least six.string_types in place of str.

dstansby added some commits Dec 22, 2016

@dstansby dstansby Use cbook string class checker abcef5a
@dstansby dstansby Avoid comparing numpy array to string when marker is an array
42786e3

dstansby changed the title from Check aspect to avoid comparing numpy array to tuple of strings to Avoid comparing numpy array to strings in two places Dec 22, 2016

Contributor

dstansby commented Dec 22, 2016

There was another similar warning coming up in in #5806 that I have also fixed.

efiring changed the title from Avoid comparing numpy array to strings in two places to [MRG+1] Avoid comparing numpy array to strings in two places Dec 22, 2016

Owner

efiring commented Dec 22, 2016

Test failures are unrelated (common random failures).

NelleV changed the title from [MRG+1] Avoid comparing numpy array to strings in two places to [MRG+2] Avoid comparing numpy array to strings in two places Dec 22, 2016

Contributor

NelleV commented Dec 22, 2016

LGTM 👍
launching the travis test again.

QuLogic changed the title from [MRG+2] Avoid comparing numpy array to strings in two places to Avoid comparing numpy array to strings in two places Dec 22, 2016

@QuLogic QuLogic merged commit b8568f9 into matplotlib:master Dec 22, 2016

2 of 3 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 62.074%
Details

QuLogic added this to the 2.1 (next point release) milestone Dec 22, 2016

dstansby deleted the dstansby:aspect-array-warning branch Dec 22, 2016

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