FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Only return a declared type from __new__ if it is a subtype of the class by msullivan · Pull Request #7656 · python/mypy · GitHub

  1. python
  2. mypy

Repository navigation

Only return a declared type from __new__ if it is a subtype of the class - #7656

Merged
msullivan merged 1 commit into
masterfrom
__new__child
Oct 9, 2019
ConversationCommits (1)ChecksFiles changed
Merged

Only return a declared type from __new__ if it is a subtype of the class#7656
msullivan merged 1 commit into
masterfrom
__new__child

Conversation

msullivan commented Oct 8, 2019

Copy link
Copy Markdown
Collaborator

Fixes #7597.

ilevkivskyi approved these changes Oct 9, 2019
View reviewed changes

ilevkivskyi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Thanks for the fix!

I think this kind of implicit logic should be documented somewhere in mypy docs, otherwise LGTM.

Comment thread test-data/unit/check-classes.test
pass

reveal_type(A()) # N: Revealed type is 'builtins.int'
reveal_type(A()) # N: Revealed type is '__main__.A'

ilevkivskyi Oct 9, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I think we should keep the declared return type for the class itself. The original PR closed an issue called "Honor return type of __new__", so this change would be a step back.

msullivan Oct 9, 2019

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

We report an error at the __new__ declaration site, and there are a bunch of other situations where we wouldn't honor it before (like if it is a union or something other than an instance or tuple), so I think this actually makes thing a little more consistent.

msullivan merged commit c95ecc1 into master Oct 9, 2019
msullivan deleted the __new__child branch October 9, 2019 20:18

actionless commented Oct 10, 2019

Copy link
Copy Markdown

thanks! it worked however i found another problem with popen inheritance, i am posting an issue right now

althonos mentioned this pull request Oct 30, 2019
Merged
4 of 9 tasks
intgr mentioned this pull request Jan 12, 2023
Closed
Viicos mentioned this pull request Apr 19, 2023
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

ilevkivskyi ilevkivskyi approved these changes
JukkaL Awaiting requested review from JukkaL

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Unexpected error when inheriting a class from Popen and overriding some method in it

3 participants

Back | FazBrowse Home | New Git URL