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

Support decorated constructors #6645

Merged
merged 5 commits into from Apr 27, 2019

Conversation

@ilevkivskyi
Copy link
Collaborator

commented Apr 8, 2019

Fixes #1706
Fixes #5398

Previously, decorated constructors either resulted in silent Any types for class objects (in case of a decorated __init__()) or incorrect signatures (in case of a decorated __new__()).

This PR adds support for decorated constructors, implementation is straightforward.

This PR supersedes #5798.

@ilevkivskyi ilevkivskyi requested review from msullivan and gvanrossum Apr 8, 2019

@gvanrossum

This comment has been minimized.

Copy link
Member

commented Apr 10, 2019

I'm sorry, I don't think I can review this.

@msullivan
Copy link
Collaborator

left a comment

This looks good!

How much damage does it cause for internal codebases?

def type_object_type_from_function(init_or_new: FuncBase,
This includes nomral functions, overloaded functions, and decorators

This comment has been minimized.

Copy link
@msullivan

msullivan Apr 11, 2019

Collaborator

typo - "nomral"

else:
assert isinstance(method.type, FunctionLike) # is_valid_constructor() ensures this
t = method.type
signature = bind_self(t)

This comment has been minimized.

Copy link
@msullivan

msullivan Apr 11, 2019

Collaborator

Any particular reason for moving this bind_self out of type_object_type_from_function? I don't have any objection, I'm just wondering if there is a technical reason I'm missing or you just like it more.

This comment has been minimized.

Copy link
@ilevkivskyi

ilevkivskyi Apr 27, 2019

Author Collaborator

No particular reason. I think I started refactoring by pulling function_type() out of there, and pulled whole line out of there. But looking again at it I think it makes sense to keep bind and map together, so I will put it back.

@ilevkivskyi

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 27, 2019

How much damage does it cause for internal codebases?

Not so much actually, around 60 errors S+C.

@ilevkivskyi ilevkivskyi merged commit b778c87 into python:master Apr 27, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@ilevkivskyi ilevkivskyi deleted the ilevkivskyi:fix-deco-init branch Apr 27, 2019

msullivan added a commit that referenced this pull request Apr 27, 2019

Fix mypyc build
PR #6645 introduced a mypyc failure due to the irritating #3603
intersection type issue.

Fix it by testing again SYMBOL_FUNCBASE_TYPES instead of FuncBase
@msullivan msullivan referenced this pull request Apr 27, 2019

ilevkivskyi added a commit that referenced this pull request Apr 27, 2019

Fix mypyc build (#6731)
PR #6645 introduced a mypyc failure due to the irritating #3603
intersection type issue.

Fix it by testing again SYMBOL_FUNCBASE_TYPES instead of FuncBase
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.