Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

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

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())

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())
```
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
@hauntsaninja hauntsaninja deleted the has_member branch February 25, 2020 20:13
msullivan added a commit that referenced this pull request Mar 6, 2020
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
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
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.
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants