-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Deprecate is_string_like, is_sequence_of_strings #8011
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
Conversation
130dead
to
54cbb67
Compare
Has this been true since NumPy 1.7.1? |
Yes.
and likewise on Py2 (but with |
Looks like an example might have broken. |
54cbb67
to
f649041
Compare
Fixed. |
I had missed that numpy's `str_` (and `unicode_` on Py2) derive from the corresponding builtin type(s), so `isinstance(x, six.string_types)` is just as good.
`Axes3D.scatter` no longer flattens a 2D color input.
rebased. bumping :-) |
means that arrays are now never num-like or string-like regardless of their | ||
dtype. | ||
`cbook.is_numlike` now only checks that its argument is an instance of | ||
``(numbers.Number, np.Number)`` and. In particular, this means that arrays are |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and what?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo, fixed
c_list = [] | ||
for c in colors: | ||
c_list.append([c]*20) | ||
c_list.extend([c] * 20) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this actually related?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not really, moved out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh actually i remember now, it is.
this is because 3d scatter no longer implicitly flattens its input (see the last hunk of the diff).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably should add that to the API what's new, then.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Also documented the similar change in #7786 at the same time.
|
||
`cbook.is_string_like` and `cbook.is_sequence_of_strings` have been | ||
deprecated. Use ``isinstance(obj, six.string_types)`` and ``all(isinstance(o, | ||
six.string_types) for o in obj) and not iterable(obj)`` instead. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That doesn't seem right:
In [9]: from matplotlib.cbook import iterable, is_sequence_of_strings
In [10]: obj = ['a', 'b', 'c']
In [11]: is_sequence_of_strings(obj)
Out[11]: True
In [12]: import six
In [13]: all(isinstance(o, six.string_types) for o in obj) and not iterable(obj)
Out[13]: False
(using 2.0.0, though)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the "not" needs to be removed, fixed
lib/matplotlib/cbook/__init__.py
Outdated
raise ValueError("First argument must be a sequence") | ||
nrecs = len(args[0]) | ||
margs = [] | ||
seqlist = [False] * len(args) | ||
for i, x in enumerate(args): | ||
if (not is_string_like(x)) and iterable(x) and len(x) == nrecs: | ||
if ((not isinstance(x, six.string_types)) and iterable(x) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can drop extra parentheses.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
lib/matplotlib/cbook/__init__.py
Outdated
@@ -1219,10 +1221,10 @@ def finddir(o, match, case=False): | |||
is True require an exact case match. | |||
""" | |||
if case: | |||
names = [(name, name) for name in dir(o) if is_string_like(name)] | |||
names = [(name, name) for name in dir(o) if isinstance(name, six.string_types)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this too long?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
lib/matplotlib/colors.py
Outdated
@@ -789,7 +789,7 @@ def __init__(self, colors, name='from_list', N=None): | |||
if N is None: | |||
N = len(self.colors) | |||
else: | |||
if (cbook.is_string_like(self.colors) and | |||
if (isinstance(self.colors, six.string_types) and | |||
cbook.is_hashable(self.colors)): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, what strings are not hashable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably was the case of masked string arrays. removed the now redundant check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there might be a few more of these that I didn't comment on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Caught two more, should be all fixed now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see it...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's only one line that changes is_hashable
there (this one.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah sorry, thought you meant overly long lines. Now I have removed the hashability checks too.
lib/matplotlib/lines.py
Outdated
@@ -1289,7 +1289,7 @@ def update_from(self, other): | |||
|
|||
def _get_rgba_face(self, alt=False): | |||
facecolor = self._get_markerfacecolor(alt=alt) | |||
if is_string_like(facecolor) and facecolor.lower() == 'none': | |||
if isinstance(facecolor, six.string_types) and facecolor.lower() == 'none': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably too long, also.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
lib/matplotlib/text.py
Outdated
if ((is_string_like(s1) and s1.split()[0] == "offset") or | ||
(is_string_like(s2) and s2.split()[0] == "offset")): | ||
if ((isinstance(s1, six.string_types) and s1.split()[0] == "offset") or | ||
(isinstance(s2, six.string_types) and s2.split()[0] == "offset")): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks a bit over as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
@@ -40,8 +39,6 @@ class BlockingInput(object): | |||
""" | |||
def __init__(self, fig, eventslist=()): | |||
self.fig = fig | |||
if not is_sequence_of_strings(eventslist): | |||
raise ValueError("Requires a sequence of event name strings") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why remove this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because it's not as if the check was really useful anyways -- one can always pass a sequence of invalid event names and it'll be just as bad as of one passed in a sequence of nonstrings.
While implementing #7996 (see also #7835), I had missed that numpy's
str_
(andunicode_
on Py2) derive from the corresponding builtin type(s), soisinstance(x, six.text_type)
is just as good. So I just inlined that and restored the old definition ofis_string_like
, immediately deprecating it.Would also (upon removal) close #7725.