Skip to content
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

funcsigs.signature() doesn't support classes on PyPy3 #11016

Closed
rlamy opened this issue Apr 23, 2018 · 5 comments
Closed

funcsigs.signature() doesn't support classes on PyPy3 #11016

rlamy opened this issue Apr 23, 2018 · 5 comments
Labels

Comments

@rlamy
Copy link
Contributor

rlamy commented Apr 23, 2018

Description

sklearn.externals.funcsigs.signature() doesn't work (i.e. raises a ValueError) with user-defined classes on PyPy3. This is the source of most of the test failures seen in #11010.

The external project funcsigs seems unmaintained and doesn't appear to have a publicly accessible repo containing the last released version, so I'm not sure how to proceed with testing and fixing this issue.

Steps/Code to Reproduce

from sklearn.externals.funcsigs import signature

class Foo:
    def __init__(self, x):
        pass

sig = signature(Foo)
print(sig.parameters)

Expected Results

On CPython 3.5:

OrderedDict([('x', <Parameter at 0x7ff90116f0e8 'x'>)])

(but note that you get a ValueError for classes that don't define __init__)

Actual Results

On pypy3.5:

Traceback (most recent call last):
  File "xx.py", line 7, in <module>
    sig = signature(Foo)
  File "/home/ronan/dev/scikit-learn/sklearn/externals/funcsigs.py", line 173, in signature
    raise ValueError('callable {0!r} is not supported by signature'.format(obj))
ValueError: callable <class '__main__.Foo'> is not supported by signature
@rth
Copy link
Member

rth commented Apr 23, 2018

The funcsigs repo seems to be https://github.com/aliles/funcsigs ; well it's a Python 3.3 backport so it probably doesn't need to be maintained too much.

On Python 3.4+ it's probably better to use from inspect import signature directly, and that's what is done in when importing signature from sklearn.utils.fixes. The latter appears to work with PyPy 3; so we just need to replace all direct imports of sklearn.externals.funcsigs with it.

Is this an issue for PyPy 2?

@rlamy
Copy link
Contributor Author

rlamy commented Apr 23, 2018

That repo only has version 0.4 from 2013 but PyPI has 1.0.2 from 2016. Anyway, inspect.signature should work better.

There seems to be an issue on pypy2 as well, but I haven't debugged it yet.

@rlamy
Copy link
Contributor Author

rlamy commented Apr 23, 2018

So, yes, there is a similar issue on pypy2. There, after modifying the example to use a new-style class, the result is:
OrderedDict([('args', <Parameter at 0x3039050 'args'>), ('keywords', <Parameter at 0x3039088 'keywords'>)])

@rth
Copy link
Member

rth commented Apr 24, 2018

So we can just make sure everything is a new-style class as a workaround? There shouldn't be too many old-style classes since e.g. anything inheriting from BaseEstimator should be a new-style class..

@rth rth added the pypy label Sep 2, 2018
@rth
Copy link
Member

rth commented Sep 2, 2018

A workaround of using new-style classes was applied in #11010, I believe. Closing.

@rth rth closed this as completed Sep 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants