Skip to content

make __class__ refer to the current object's class #1549

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

Merged
merged 2 commits into from
Sep 28, 2017

Conversation

JelleZijlstra
Copy link
Member

@matthiaskramm
Copy link
Contributor

Happy to merge this, but I think you'll need to resolve conflicts first.

@JelleZijlstra
Copy link
Member Author

Thanks! Hadn't noticed there was a merge conflict.

@matthiaskramm matthiaskramm merged commit 1a164b6 into python:master Sep 28, 2017
@gvanrossum
Copy link
Member

I wonder if we should revert this? I just checked our internal codebases against this and I found some mysterious new mypy errors all complaining about the use of self.__class__. I don't have time to look into it but FTR here are some of the errors:

[...]/task.py:221: error: Cannot determine type of '__name__'
[...]/destroyable_base.py:550: error: Incompatible types in assignment (expression has type "Type[SuperDestroyable[_DClass, _ActClass]]", variable has type "Type[_DClass]")

The code for the first was (in a class derived from threading.local):

        if SupportedTrackedTaskWorkers.is_supported(self.__class__.__name__, self.task_name):

and for the second (in a generic class):

        concrete_class = self.__class__  # type: Type[_DClass]

Let me know if you have more questions about context.

@JelleZijlstra
Copy link
Member Author

I'm OK with reverting for now (#1632). It's not obvious to me where these errors are coming from but I'll look into it.

gvanrossum pushed a commit that referenced this pull request Sep 29, 2017
…1632)

This reverts commit 1a164b6.

Reverts #1549.

See Guido's comments in the original PR.
@gvanrossum
Copy link
Member

I don't understand either. I should add that the second one perhaps gives a better clue, as it appears inside the __init__ of a class declared as follows:

_DClass = TypeVar('_DClass', bound='SuperDestroyable')
_ActClass = TypeVar('_ActClass')

class SuperDestroyable(Generic[_DClass, _ActClass]):

@JelleZijlstra
Copy link
Member Author

I wonder if the second one is actually a bug in your code. self.__class__ isn't a Type[_DClass], so the assignment is unsafe.

@gvanrossum
Copy link
Member

Oh, interesting. I'll ask the author of the code -- it seems _DClass is actually intended to be a subclass of the given SuperDestroyable instantiation, which would make the type declaration exactly what's intended, but then I'm at a loss why previously it passed (when __class__ was just a type). And it's a downcast so it should require a cast() instead of a # type: annotation.

@gvanrossum
Copy link
Member

gvanrossum commented Sep 29, 2017

I'm also going to try and boil down the first. I came up with this, and while it doesn't give the same error, it does seem relevant:

import threading

class C(threading.local):

    def foo(self) -> None:
        reveal_type(self)  # C
        reveal_type(self.__class__)  # Type[C]
        reveal_type(self.__class__.__name__)  # Any ?!?!

The actual production code shows the same phenomenon.

When I remove the threading.local base class (from my toy example), the type of self.__class__.__name__ becomes str.

So somehow the weirdness in the definition of threading.local (it's declared as type Any) seems to affect this.

@gvanrossum
Copy link
Member

I have to punt on this for now. I'm going to merge the typeshed sync and cut the release branch now.

gvanrossum added a commit to python/mypy that referenced this pull request Sep 29, 2017
This is to pick up the revert of python/typeshed#1549.
@ilevkivskyi
Copy link
Member

@gvanrossum

but then I'm at a loss why previously it passed (when __class__ was just a type). And it's a downcast so it should require a cast() instead of a # type: annotation.

type annotation is interpreted as Type[Any] (not Type[object]) by is_subtype and friends, so that assignment is considered safe by mypy:

t: type
x: Type[int] = t  # this is considered safe because of implicit Any

When I remove the threading.local base class (from my toy example), the type of self.__class__.__name__ becomes str.
So somehow the weirdness in the definition of threading.local (it's declared as type Any) seems to affect this.

analyze_member_access for TypeType is still broken. There was an attempt to fix it in PR python/mypy#2832, but it is still incomplete and it turns out it introduced an --incremental crash, see second traceback in python/mypy#3852 that I am fixing now. I will also add these use cases as tests to my PR.

ilevkivskyi pushed a commit to ilevkivskyi/typeshed that referenced this pull request Mar 7, 2018
msullivan added a commit that referenced this pull request Sep 26, 2018
msullivan added a commit that referenced this pull request Sep 26, 2018
yedpodtrzitko pushed a commit to yedpodtrzitko/typeshed that referenced this pull request Jan 23, 2019
@JelleZijlstra JelleZijlstra deleted the dunderclass branch May 30, 2020 04:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants