Skip to content

regression in py38 with bool-as-index deprecation warning #14397

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.

Sign up for GitHub

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

Jump to bottom
Open
tacaswell opened this issue Aug 29, 2019 · 8 comments
Open

regression in py38 with bool-as-index deprecation warning #14397

tacaswell opened this issue Aug 29, 2019 · 8 comments

Comments

@tacaswell
Copy link
Contributor

Also reported upstream at https://bugs.python.org/issue37980

With numpy 1.17.1 and python37 the following does not warn

sorted([1, 2], reverse=np.bool_(True))

but with py38

In [2]: sorted([1, 2], reverse=np.bool_(True))                                                                                                                  
<ipython-input-2-6726f33270df>:1: DeprecationWarning: In future, it will be an error for 'np.bool_' scalars to be interpreted as an index
  sorted([1, 2], reverse=np.bool_(True))
Out[2]: [2, 1]

This bisects to python/cpython#11952 which use __index__ in more places which is tripping the deprecation in a surprising place.

sorted([1, 2], reverse=bool(np.bool_(True)))

Works in all cases.
@eric-wieser
Copy link
Member

This looks like a cpython bug, i'd expect them to call __bool__ there

@seberg
Copy link
Member

seberg commented Aug 29, 2019

Hmmpf, as much as I like that they use __index__ now, this is a bit annoying. I think for bool using unsafe coercion is probably just OK in any case? Of course from pythons perspective our bool should implement __index__, but there are good reasons why we don't.

For now my gut feeling is to agree with Eric and think that python should probably leave bool coercion unsafe, its not like this also forbids bool(19283989123) anyway.

@seberg
Copy link
Member

seberg commented Aug 29, 2019

It is a bit strange anyway? If they change bool() here, than how am I supposed to coerce to bool? Using operator.nonzero?

@QuLogic
Copy link
Contributor

QuLogic commented Aug 31, 2019

We see this in Matplotlib rebuilds for Python 3.8 on Fedora Rawhide, as it is triggering failures in tests that expect a certain number of warnings.

@eric-wieser
Copy link
Member

I think we should report this upstream, preferably with a non-numpy repro

@tacaswell
Copy link
Contributor Author

From the Matpltolib side, I put in matplotlib/matplotlib#15168 I will leave the discussion on https://bugs.python.org/issue37980 to numpy and core developers.

@amueller amueller mentioned this issue Nov 15, 2019
Merged
@amueller
Copy link

Also creating failures in scikit-learn (as we expect no deprecation warnings in our own code).

@seberg
Copy link
Member

seberg commented Jan 20, 2024 •
edited
Loading

Stumbled on this, should we just back down for now (it would be nice to do this, because we reject our bools as integers in many places and have to do so)?
(And if so, maybe someone can make look into it even?)

Python 3.10 at least still doesn't like it, and while I think it is a Python problem at the core, I don't see how they can fix it easily.
(Basically, I think they would have to add a __opt_bool__ (or maybe allow subclassing, their bool, so that sorting could evolve to using that).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@seberg @tacaswell @QuLogic @eric-wieser @amueller