Skip to content
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

Deprecate is_string_like #7835

Closed
anntzer opened this issue Jan 15, 2017 · 7 comments
Closed

Deprecate is_string_like #7835

anntzer opened this issue Jan 15, 2017 · 7 comments
Milestone

Comments

@anntzer
Copy link
Contributor

anntzer commented Jan 15, 2017

This issue is similar to #7795 ("is_numlike should be deprecated and replaced by the correct isinstance calls"): cbook.is_string_like returns True for 0-dimensional masked arrays of string dtype (but not regular arrays...), but testing for is_string_like is often followed by calling of string methods (.lower(), etc.) on the object, or passing the object to, say, open() -- neither of which work with masked arrays of string dtype. (check yourself with git grep -A2 'is_string_like(')

In other words the actual semantics of the function simply do not match how it is used.

Most of these calls should probably be replaced by isinstance(obj, six.string_types) (possibly isinstance(obj, (six.string_types, six.binary_type)) depending on the case), which is much more explicit.

@tacaswell tacaswell added this to the 2.2 (next next feature release) milestone Jan 15, 2017
@tacaswell
Copy link
Member

Why is 'isinstance' checking better than improving the duck typing?

@anntzer
Copy link
Contributor Author

anntzer commented Jan 15, 2017

Because I doubt you will ever be able to make it possible to pass 0d-masked-arrays to open(), and that it'll at least take a while before masked string arrays implement string methods (as methods, not as standalone functions), if they ever do (which I doubt).

@dopplershift
Copy link
Contributor

Isn't the purpose of having a is_string_like function the ability to abstract away the way we're doing the check (i.e. duck-typing vs. isinstance)?

@anntzer
Copy link
Contributor Author

anntzer commented Jan 16, 2017

open doesn't care about abstraction. It exactly wants a string, or a string subclass.
Likewise, calling string methods (via method syntax) doesn't work on numpy arrays of string dtype, so the abstraction is not very good.

@dopplershift
Copy link
Contributor

Do we really have no uses of is_string_like that could actually us a duck string?

@anntzer
Copy link
Contributor Author

anntzer commented Jan 16, 2017

There may well be some -- but my guess is (and a quick grep suggests) that they are a minority, so I'd suggest looking at them and asking whether we really need to support them (note that as mentioned above, so far numpy 0d arrays of string dtype were actually rejected, only 0d masked arrays of string dtype were accepted, and no one seemed to complain.)

I did just realize that indexing numpy string arrays returns a np.str_, which is a different type, but similarly to the is_numlike case, one can just define string_like = (str, np.str_) and then use isinstance(obj, string_like) (yes, this could even be the new implemetation of is_string_like if you want to keep it...).

@anntzer
Copy link
Contributor Author

anntzer commented Apr 22, 2017

Closed by #8011.

@anntzer anntzer closed this as completed Apr 22, 2017
@QuLogic QuLogic modified the milestones: 2.1 (next point release), 2.2 (next next feature release) May 3, 2017
dbrnz added a commit to dbrnz/matplotlib that referenced this issue Aug 24, 2017
maweigert added a commit to maweigert/matplotlib-scalebar that referenced this issue Oct 9, 2017
p-leger added a commit to p-leger/gtfspy that referenced this issue Jul 17, 2019
With version 2.1 the function 'is_string_like' was deprecated, with
version 3.0.0 it was removed entirely.

* matplotlib/matplotlib#7835
* https://matplotlib.org/3.0.0/api/api_changes.html
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants