Skip to content

Incorrect uses of is_numlike #7795

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

Closed
anntzer opened this issue Jan 10, 2017 · 6 comments · Fixed by #10468
Closed

Incorrect uses of is_numlike #7795

anntzer opened this issue Jan 10, 2017 · 6 comments · Fixed by #10468
Milestone

Comments

@anntzer
Copy link
Contributor

anntzer commented Jan 10, 2017

cbook.is_numlike(obj) is implemented by checking whether obj + 1 raises. In particular, this means that numpy numeric arrays are considered as numlikes, which appears to be unintended in most cases.

As an example (not the only one), while

plt.scatter([0], [0], marker=[[0, 0], [0, 1], [1, 1]])

works as expected,

plt.scatter([0], [0], marker=np.array([[0, 0], [0, 1], [1, 1]]))

raises an exception because code in markers checks (effectively, in this case) for whether markers is "numlike", and, if so, considers it to be a number of sides for a polygon marker (at the beginning of _set_tuple_marker).

In my never-ending push to get rid of as much of cbook as possible :-), I would suggest replacing calls to is_numlike to either np.issubdtype(type(obj), np.number) (if we were using numpy>=1.9.0, this could be isinstance(obj, abc.Number)) or, if it was actually intended to accept arrays, np.issubtype(np.asarray(obj).dtype, np.number) -- we may as well distinguish explicitly between the two cases.

Edit: Actually one can just define somewhere Number = (abc.Number, np.number), then isinstance(obj, Number) will work even for older numpys (I guess).

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

Would that correctly handle a 0d (numpy-scalar) array case?

@anntzer
Copy link
Contributor Author

anntzer commented Jan 11, 2017

numpy scalars and numpy 0d arrays are not the same:

n [1]: np.float64(0), np.array(np.float64(0))
Out[1]: (0.0, array(0.0))

Scalars pass both tests, 0d arrays only pass the second one, but I don't think we particularly need to support 0d arrays, given that you're unlikely to create them by accident.

@jklymak
Copy link
Member

jklymak commented Jan 18, 2018

Is this still open given #7996?

@anntzer
Copy link
Contributor Author

anntzer commented Jan 18, 2018

I would still like to ultimately replace is_numlike by isinstance(..., Number) but that needs to wait until we bump the min numpy version to 1.9.

@jklymak jklymak modified the milestones: v2.2, v3.0 Jan 18, 2018
@jklymak
Copy link
Member

jklymak commented Jan 18, 2018

Which I assume won't happen until 3.0. Feel free to bump back...

@QuLogic
Copy link
Member

QuLogic commented Feb 15, 2018

Minimum version is 1.10 in master now, so that can be implemented now.

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

Successfully merging a pull request may close this issue.

4 participants