gh-93884: Improve test coverage of PyNumber_ToBase

[CharlieZhao95]

Link to #93884

• Test with some large negative and positive values(out of range of a longlong,i.e.[-2**63, 2**63-1])
• Test with objects of non-int type

• Issue: Incomplete test coverage of PyNumber_ToBase #93884

Automerge-Triggered-By: GH:mdickinson

[JustAnotherArchivist]

Because TrapInt is a subclass of int, this never actually calls __index__. PyNumber_Index uses PyLong_Check internally and simply returns the input object if it passes that (plus later converting it to an actual int). PyLong_Check(o) is effectively isinstance(o, int). While it might be worth testing that conversion of subclasses, that's something for the index tests. Here, we need an object that isn't an int subclass.

>>> class TrapInt(int):
...  def __index__(self):
...   print("index")
...   return int(self)
... 
>>> # Explicitly calling it calls __index__, of course:
>>> TrapInt(1).__index__()
index
1
>>> # But operator.index (= PyNumber_Index) does not:
>>> operator.index(TrapInt(42))
42

[CharlieZhao95]

Right. The TrapInt class is meaningless for testing PyNumber_ToBase, it behaves the same way as int. I would like to replace TrapInt with the following class.

class IDX(object):
    def __init__(self, val):
        self.val = val
    def __index__(self):
        return self.val

Also, I looked at the docs of PyNumber_ToBase(PyObject *n, int base), which states that If *n* is not a Python int, it is converted with PyNumber_Index first. Wouldn't it be better to change this sentence to If *n* is not a Python int or subclass of int, ...

[JustAnotherArchivist]

Yes, that class is what I'd do (although the (object) bit is unnecessary with Py3; all classes are based on object).

Regarding the suggested doc change: I'm not sure that's necessary. 'If x is a type' should be understood as isinstance(x, type). See for example the docs of PyLong_AsLong et al. Functions which expect only exactly the specified type mention that explicitly, e.g. PyLong_CheckExact.

However, I see that several of the PyNumber_* function descriptions use the Python object names (float and int) rather than the C names (PyFloatObject and PyLongObject) as is otherwise used in much of the C API documentation. That might be worth considering changing.

[JustAnotherArchivist]

LGTM

[MYavuzYAGIS]

LGTM!

[mdickinson]

LGTM

[miss-islington]

Status check is done, and it's a failure ❌.

[mdickinson]

Status check is done, and it's a failure ❌.

Not quite sure what happened there. But all of the checks look as though they passed.

[DanielNoord]

Not quite sure what happened there. But all of the checks look as though they passed.

@mdickinson Just to let you know. This bug was introduced in python/miss-islington#565 but I have provided a fix in python/miss-islington#576.

Sorry for the confusion! Stupid mistake by me 😅

[mdickinson]

@DanielNoord Thanks!