8000 BUG: number.__int__ and int(number) are not the same · Issue #9972 · numpy/numpy · GitHub
[go: up one dir, main page]

Skip to content

BUG: number.__int__ and int(number) are not the same #9972

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
eric-wieser opened this issue Nov 6, 2017 · 9 comments
Closed

BUG: number.__int__ and int(number) are not the same #9972

eric-wieser opened this issue Nov 6, 2017 · 9 comments

Comments

@eric-wieser
Copy link
Member
eric-wieser commented Nov 6, 2017

As worked around in #9181, we have two implementation of every number.__int__, one in scalarmath (int), and one in scalartypes (__int__).

This happens because after calling PyType_Ready, we reassign tp_number, but the __int__ slot wrapper has already been created.

It's super misleading that this is the case, and exposes cpython implementation details.

It seems to me it would be safer to not set __int__ at all until we fix this, rather than having two possibly different implementation to trap the end user

@mattip
Copy link
Member
mattip commented Nov 15, 2017

Not only because of reassignment after PyType_Ready, also because of CPython quirks. For example, this code has different paths due to CPython not inheriting nb_int but inheriting __int__ via mro lookup.

It successfully calls int(a) via int_new which eventually calls int_from_string in cpython2.7 abstract.c since PyStringArrType_Type.tp_from_number.nb_int is NULL.

It fails with a.__int__() since type(a).mro() hits numpy.generic which has gentype_int as its nb_int

>>> a = np.array(['100'])[0]
>>> int(a)
100
>>> a.__int__()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: don't know how to convert scalar number to int

@mattip
Copy link
Member
mattip commented Nov 15, 2017

Just to clarify - PyStringArrType_Type.tp_as_number is not overwritten after PyType_Ready in addscalar_math, only the numeric types are overwritten, so this is purely a CPython method lookup quirk

@eric-wieser
Copy link
Member Author

I'm wondering if the culprit is ndarray.__int__, which for some reason doesn't fall back on PyNumber_Int, instead trying to call the members directly.

@mattip
Copy link
Member
mattip commented Nov 17, 2017

That would add another layer of abstraction, slowing down the conversion for the 99% of the cases when a nb_int exists

@eric-wieser
Copy link
Member Author
eric-wieser commented Nov 17, 2017

Right, but it also means that our implementation right now is incorrect. int(x) is also supposed to try __trunc__ if __int__ doesn't exist. #10042 fixes that.

@eric-wieser
Copy link
Member Author
eric-wieser commented Nov 17, 2017

Another example:

>>> a = np.array(None)
>>> a[()] = bytearray('10')
>>> int(a)
TypeError: cannot convert to an int; scalar object is not a number
>>> int(bytearray('10'))
10

eric-wieser added a commit to eric-wieser/numpy that referenced this issue Nov 17, 2017
Rather than assuming that any object array is self-referencing, we can just use PyEnter_RecursiveCall in:

* `__int__`
* `
8000
__float__`
* `__long__`
* `__hex__`
* `__oct__`

This works towards (but does _not_ fix) numpy#9972, by not directly touching the `nb_*` slots ourselves.

Substantial code deduplication here. Error message is different, but perhaps also better:

```python
>>> int(np.array([None]))
TypeError: int() argument must be a string, a bytes-like object or a number, not 'NoneType' # now
TypeError: cannot convert to an int; scalar object is not a number #before
```
@mattip
Copy link
Member
mattip commented Feb 20, 2019

#10042 fixed these. These now work

>>> a = np.array([b'100'])[0]
>>> int(a)
100
>>> a.__int__()
100

and

>>> a = np.array(None)
>>> a[()] = bytearray(b'10')
>>> int(a)
10

@bsipocz
Copy link
Member
bsipocz commented Oct 15, 2019

It looks like everything works here now. Can this be closed @mattip?

@mattip
Copy link
Member
mattip commented Dec 8, 2019

Closing. Please reopen if more discrepancies appear

@mattip mattip closed this as completed Dec 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants
0