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.

Sign up for GitHub

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

Jump to bottom

mypy: remove has_member #8438

Merged
merged 1 commit into from Feb 25, 2020
Merged

mypy: remove has_member #8438

merged 1 commit into from Feb 25, 2020
+2 −48

Conversation

hauntsaninja
Copy link
Collaborator

In particular:

  • The test case mentioned in the code passes without it
  • The test case changed seems to have more desirable behaviour now,
    consider:
from typing import Any

"""
class C:
    def __radd__(self, other) -> float:
        return 1.234
"""
C: Any
class D(C):
    pass

reveal_type("str" + D())

mypy: remove has_member
5114077 
In particular:
- The test case mentioned in the code passes without it
- The test case changed seems to have more desirable behaviour now,
  consider:

```
from typing import Any

"""
class C:
    def __radd__(self, other) -> float:
        return 1.234
"""
C: Any
class D(C):
    pass

reveal_type("str" + D())
```
JukkaL
JukkaL approved these changes Feb 25, 2020
View reviewed changes
Copy link
Collaborator

@JukkaL JukkaL left a comment

Choose a reason for hiding this comment

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

Thanks! I agree that it looks like has_member can now be safely removed.

@Michael0x2a In case you think that the comment is still valid and this isn't something we should do, we can reconsider this change.

@JukkaL JukkaL merged commit 09cdab4 into python:master Feb 25, 2020
@Michael0x2a
Copy link
Collaborator

If all the tests are now passing/the metaclass thing no longer seems to be an issue, I agree merging this seems like a good idea.

I do remember filing an issue related to this in #5491 -- it could be worth double-checking to see if we also still need to keep that open.

sthagen added a commit to sthagen/python-mypy that referenced this pull request Feb 25, 2020
@sthagen
Merge pull request #29 from python/master
363796b 
mypy: remove has_member (python#8438)
@hauntsaninja hauntsaninja deleted the has_member branch February 25, 2020 20:13
msullivan added a commit that referenced this pull request Mar 6, 2020
@msullivan
Revert "mypy: remove has_member (#8438)"
52af459 
It turns out that the has_member check is an important (accidental?)
performance optimization. Removing this caused a major (30+%?)
slowdown at dropbox. There might be a better way to optimize this but
I'm just going to revert it for now at least.

This reverts commit 09cdab4.
ilevkivskyi pushed a commit that referenced this pull request Mar 6, 2020
@msullivan
Revert "mypy: remove has_member (#8438)" (#8500)
514dbfb 
It turns out that the has_member check is an important (accidental?)
performance optimization. Removing this caused a major (30+%?)
slowdown at dropbox. There might be a better way to optimize this but
I'm just going to revert it for now at least.

This reverts commit 09cdab4.
msullivan added a commit that referenced this pull request Mar 6, 2020
@msullivan
Revert "mypy: remove has_member (#8438)" (#8500)
dc0d35f 
It turns out that the has_member check is an important (accidental?)
performance optimization. Removing this caused a major (30+%?)
slowdown at dropbox. There might be a better way to optimize this but
I'm just going to revert it for now at least.

This reverts commit 09cdab4.
@hauntsaninja hauntsaninja mentioned this pull request Nov 28, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JukkaL JukkaL JukkaL approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@hauntsaninja @Michael0x2a @JukkaL