stubtest: incorrect runtime signature of cython `__cinit__` methods

[jorenham]

Originally posted in #19307 (comment) (slightly edited):

---

I'm seeing a bunch of new errors pop up when running stubtest on scipy-stubs using mypy master (ffe2db8).

All errors are seem to be related to __cinit__ Cython methods:

• scipy.io.matlab._mio5_utils.VarReader5 (not public api)
  • scipy: https://github.com/scipy/scipy/blob/7d48c99615028935614943007fe61ce361dddebf/scipy/io/matlab/_mio5_utils.pyx#L169
  • scipy-stubs: https://github.com/scipy/scipy-stubs/blob/0ecf76215d341637211681567c22aefdd210de8f/scipy-stubs/io/matlab/_mio5_utils.pyi#L53
• scipy.stats._unuran.unuran_wrapper.TransformedDensityRejection, re-exported as scipy.stats.sampling.TransformedDensityRejection
  • scipy: https://github.com/scipy/scipy/blob/7d48c99615028935614943007fe61ce361dddebf/scipy/stats/_unuran/unuran_wrapper.pyx#L815-L825
  • scipy-stubs: https://github.com/scipy/scipy-stubs/blob/0ecf76215d341637211681567c22aefdd210de8f/scipy-stubs/stats/_unuran/unuran_wrapper.pyi#L64-L77
• scipy.stats._unuran.unuran_wrapper.SimpleRatioUniforms, re-exported as scipy.stats.sampling.SimpleRatioUniforms
  • scipy: https://github.com/scipy/scipy/blob/7d48c99615028935614943007fe61ce361dddebf/scipy/stats/_unuran/unuran_wrapper.pyx#L1108-L1115
  • scipy-stubs: https://github.com/scipy/scipy-stubs/blob/0ecf76215d341637211681567c22aefdd210de8f/scipy-stubs/stats/_unuran/unuran_wrapper.pyi#L89-L100
• scipy.stats._unuran.unuran_wrapper.NumericalInversePolynomial, re-exported as scipy.stats.sampling.NumericalInversePolynomial
  • scipy: https://github.com/scipy/scipy/blob/7d48c99615028935614943007fe61ce361dddebf/scipy/stats/_unuran/unuran_wrapper.pyx#L1382-L1390
  • scipy-stubs: https://github.com/scipy/scipy-stubs/blob/0ecf76215d341637211681567c22aefdd210de8f/scipy-stubs/stats/_unuran/unuran_wrapper.pyi#L103-L113
• scipy.stats._unuran.unuran_wrapper.NumericalInverseHermite, re-exported as scipy.stats.sampling.NumericalInverseHermite
  • scipy: https://github.com/scipy/scipy/blob/7d48c99615028935614943007fe61ce361dddebf/scipy/stats/_unuran/unuran_wrapper.pyx#L1946-L1953
  • scipy-stubs: https://github.com/scipy/scipy-stubs/blob/0ecf76215d341637211681567c22aefdd210de8f/scipy-stubs/stats/_unuran/unuran_wrapper.pyi#L127-L138
• scipy.stats._unuran.unuran_wrapper.DiscreteAliasUrn, re-exported as scipy.stats.sampling.DiscreteAliasUrn
  • scipy: https://github.com/scipy/scipy/blob/7d48c99615028935614943007fe61ce361dddebf/scipy/stats/_unuran/unuran_wrapper.pyx#L2370-L2375
  • scipy-stubs: https://github.com/scipy/scipy-stubs/blob/0ecf76215d341637211681567c22aefdd210de8f/scipy-stubs/stats/_unuran/unuran_wrapper.pyi#L149-L157
• scipy.stats._unuran.unuran_wrapper.DiscreteGuideTable, re-exported as scipy.stats.sampling.DiscreteGuideTable
  • scipy: https://github.com/scipy/scipy/blob/7d48c99615028935614943007fe61ce361dddebf/scipy/stats/_unuran/unuran_wrapper.pyx#L2628-L2633
  • scipy-stubs: https://github.com/scipy/scipy-stubs/blob/0ecf76215d341637211681567c22aefdd210de8f/scipy-stubs/stats/_unuran/unuran_wrapper.pyi#L160-L168

Each parameter is reported as a separate error, and aliases are counted double, which causes 75 errors to be reported here.

I must admit that I know very little about cython and the C-side of cpython, so I'm not able to see why this is happening. But the fact that these only occur in case of these __cinit__ methods is kinda sus afaik.

Oh and here are the errors: https://gist.github.com/jorenham/3d41e79607bcc1f45267acc1566833d2

This might also be relevant infomation:

>>> from scipy.stats._unuran.unuran_wrapper import TransformedDensityRejection
>>> TransformedDensityRejection.__init__.__text_signature__
'($self, /, *args, **kwargs)'

So it's kinda odd that stubtest says it's def (self) at runtime, right?

[brianschubert]

cc @tungol

Copying my comment from #19307 (comment):

Hmm, your issue seems to be due to stubtest looking at TransformedDensityRejection.__init__.__objclass__ for signature information, which it expects to be TransformedDensityRejection, but is actually object at runtime:

>>> from scipy.stats._unuran.unuran_wrapper import TransformedDensityRejection
>>> TransformedDensityRejection.__init__.__objclass__
<class 'object'>

For comparison, this is what happens for stdlib C extension classes:

>>> import pickle
>>> pickle.Pickler.__init__.__objclass__
<class '_pickle.Pickler'>

and for mypyc native classes:

>>> import mypy.types
>>> mypy.types.Instance.__init__.__objclass__
<class 'mypy.types.Instance'>

So, it seems like there's either something funny with Cython extension classes, or a wrong assumption was made in #18259.

[tungol]

Thanks for the ping, @brianschubert. I've never been a Cython developer, so I can't speak to how that interacts with all this, and I don't know anything about how __cinit__ compiles into python.

That said, I'm the author of the change to stubtest which is likely causing your change: #18259.

My experience there is that this happens when your stub defines __init__ but the class is actually using a custom __new__.

>>> from scipy.io.matlab._mio5_utils import VarReader5
>>> from scipy.stats import sampling
>>> VarReader5.__init__.__objclass__
<class 'object'>
>>> VarReader5.__new__.__self__
<class 'scipy.io.matlab._mio5_utils.VarReader5'>
>>> sampling.TransformedDensityRejection.__init__.__objclass__
<class 'object'>
>>> sampling.TransformedDensityRejection.__new__.__self__
<class 'scipy.stats._unuran.unuran_wrapper.TransformedDensityRejection'>
>>> sampling.SimpleRatioUniforms.__init__.__objclass__
<class 'object'>
>>> sampling.SimpleRatioUniforms.__new__.__self__
<class 'scipy.stats._unuran.unuran_wrapper.SimpleRatioUniforms'>
>>> sampling.NumericalInversePolynomial.__init__.__objclass__
<class 'object'>
>>> sampling.NumericalInversePolynomial.__new__.__self__
<class 'scipy.stats._unuran.unuran_wrapper.NumericalInversePolynomial'>
>>> sampling.NumericalInverseHermite.__init__.__objclass__
<class 'object'>
>>> sampling.NumericalInverseHermite.__new__.__self__
<class 'scipy.stats._unuran.unuran_wrapper.NumericalInverseHermite'>
>>> sampling.DiscreteAliasUrn.__init__.__objclass__
<class 'object'>
>>> sampling.DiscreteAliasUrn.__new__.__self__
<class 'scipy.stats._unuran.unuran_wrapper.DiscreteAliasUrn'>
>>> sampling.DiscreteGuideTable.__init__.__objclass__
<class 'object'>
>>> sampling.DiscreteGuideTable.__new__.__self__
<class 'scipy.stats._unuran.unuran_wrapper.DiscreteGuideTable'>

The difference in practice is subtle, but there are scenarios involving subclassing where this matters. See my comment at #18259 (comment) for an example. I believe your new stubtest errors are correct.

from math import exp
from scipy.stats.sampling import TransformedDensityRejection

class StandardNormal:
    def pdf(self, x: float) -> float:
        # note that the normalization constant isn't required
        return exp(-0.5 * x * x)

    def dpdf(self, x: float) -> float:
        return -x * exp(-0.5 * x * x)

class HasNew(TransformedDensityRejection):
    def __new__(cls, dist):
        super().__new__(cls, dist)

HasNew(StandardNormal())  # No error

class HasInit(TransformedDensityRejection):
    def __init__(self, dist):
        super().__init__(dist)

HasInit(StandardNormal())
# Traceback (most recent call last):
#   File "test.py", line 22, in <module>
#     HasInit(StandardNormal())
#     ~~~~~~~^^^^^^^^^^^^^^^^^^
#   File "test.py", line 20, in __init__
#     super().__init__(dist)
#     ~~~~~~~~~~~~~~~~^^^^^^
# TypeError: object.__init__() takes exactly one argument (the instance to initialize)

[tungol]

For comparison, this is what happens for stdlib C extension classes:

>>> import pickle
>>> pickle.Pickler.__init__.__objclass__
<class '_pickle.Pickler'>

For the record, not all stdlib C classes work this way. For example:

>>> from itertools import cycle
>>> cycle.__init__.__objclass__
<class 'object'>
>>> cycle.__new__.__self__
<class 'itertools.cycle'>

[bzoracler]

IIRC Cython compiles a class with __cinit__[**P](self, /, *args: P.args, **kwargs: P.kwargs) -> c_void by replacing it with something approximately like this if it were written in Python ...

def __new__[**P](cls, /, *args: P.args, **kwargs: P.kwargs) -> Self:
    self = object.__new__(cls)
    for Base in reversed(cls.mro()):
        if "__cinit__" in Base.__dict__:
            Base.__cinit__(self, *args, **kwargs)
    return self

... with the caveat that after compilation, __cinit__ itself is not directly accessible from the Python runtime, so the body of the function there only runs in C.

I don't think you're allowed to write an actual __new__ these days on a Cython-compiled class; IIRC a very long time ago __new__ was reserved by Cython to do class construction, but in a very different manner to Python's __new__. Also, there is no __init__ automatically generated on a Cython-compiled class unless you explicitly define one; __cinit__ does not generate an __init__, and IMO it's misleading to write an __init__ in the stubs if the Cython class doesn't have one, as super().__init__ will definitely not do what someone thinks it does.

A glimpse of __cinit__ behaviour is described in the docs, here's a start: Cython - Fast instantiation

[brianschubert]

Thanks for the explanation @tungol! I agree that stubtest seems to be working as expected (and I'll put my dollar in the forgot about __new__ jar :-).

Hmm, this seems like it could be a common issue for downstream stubtest users. While the error is technically correct, it's pretty opaque and doesn't give users an obvious path for how to fix it.

I wonder if we could use some heuristic to detect situations like this and add a note warning stub authors to look out for __init__ vs __new__ discrepancies? Say, if the runtime __qualname__ doesn't match the stub path, or when the runtime object is object.__init__ exactly?

[tungol]

That makes sense to me. In the situation where we do have a __text_signature__ to go on, these errors will come paired with a corresponding error for __new__, which is a little more obvious what's going on:

error: itertools.cycle.__init__ is inconsistent, runtime does not have parameter "iterable"
Stub: in file typeshed/stdlib/itertools.pyi:43
def (itertools.cycle[_T`1], typing.Iterable[_T`1])
Runtime:
def (self)

error: itertools.cycle.__new__ is inconsistent, stub does not have parameter "iterable"
Stub: in file typeshed/stdlib/itertools.pyi:118
def [Self] (cls: type[Self`0]) -> Self`0
Runtime:
def (cls, iterable, /)

In this case, we don't have that, so while stubtest knows that the stubs for __init__ are wrong, it doesn't know what __new__ should be one way or the other.

When runtime is object.__init__ exactly is probably the most common case here. I pushed up an MR to add a little extra guidance in that scenario: #19733

[jorenham]

Thanks for looking into this @tungol! In hindsight, I kinda feel stupid for not trying __new__ not haha. However, it is a bit odd that __cinit__ doesn't become __init__ but a __new__, considering its name :P.

[jorenham]

I opened scipy/scipy-stubs#847 which seems to fix the stubtest errors on mypy master. Thanks again for the investigation, and feel free to close this :)