-
Notifications
You must be signed in to change notification settings - Fork 13
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
Use a more compatable way to get wrapper_descriptor and method_wrapper #30
base: master
Are you sure you want to change the base?
Conversation
The original code use type(type.__call__) to get the type `wrapper_descriptor` and use `type(all.__call__)` to get type `method_wrapper`. according the comment in here: https://github.com/Daetalus/funcsigs/blob/master/funcsigs/__init__.py#L170. It could avoid infinite recursive or sefault. In Pyston, the type of `type.__call__` is `instancemethod`. So it will cause segfault. I change it to `type(bytearray.__add__)`, because in Pyston, its type is `wrapper_descriptor`, this is also work in CPython 2.x and CPython 3.x. More information, please see scikit-learn/scikit-learn#7162.
_WrapperDescriptor = type(type.__call__) | ||
_MethodWrapper = type(all.__call__) | ||
_WrapperDescriptor = type(bytearray.__add__) | ||
_MethodWrapper = type(u'str'.__add__) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not plain 'str'.__add__
? u''
introduces an incompatibility with Python 3.2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Emm... This is because in Pyston, the type of 'str'.__add__
still is instancemethod
.
Pyston v0.6.0 (rev 8dd1298946b5dbe2b28f75a7a6d2de6486765438), targeting Python 2.7.7
>> type('str'.__add__)
<type 'instancemethod'>
>> type(u'str'.__add__)
<type 'method-wrapper'>
And according the description of funcsigs in README file, I thought we can drop the Python 3.2 support.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fine by me :-)
Guys? @epsy @msabramo @rbtcollins Does there has a better way to do this? |
(I'm only a user of funcsigs, I don;t have merge approval access) I suppose it's entirely fair to drop Python 3.2 support at this point. |
Thanks for review it! @epsy |
ping @rbtcollins maybe. |
The original code use type(type.call) to get the type
wrapper_descriptor
and usetype(all.__call__)
to get typemethod_wrapper
. according the comment in here:https://github.com/Daetalus/funcsigs/blob/master/funcsigs/__init__.py#L170.
It could avoid infinite recursive or sefault.
In Pyston, the type of
type.__call__
isinstancemethod
. So it willcause segfault. I change it to
type(bytearray.__add__)
, because inPyston, its type is
wrapper_descriptor
, this is also work in CPython2.x and CPython 3.x.
More information, please see scikit-learn/scikit-learn#7162.