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

Make logic around bind_self() consistent in different code paths #8021

merged 2 commits into from Nov 27, 2019


Copy link

ilevkivskyi commented Nov 27, 2019

Fixes #8020

There is a bunch of code/logic duplication around bind_self(), mostly because of #7724. This PR makes all three main code paths consistently follow the same structure:

  1. freshen_function_type_vars()
  2. bind_self()
  3. expand_type_by_instance(..., map_instance_to_supertype()) (a.k.a map_type_from_supertype())

I briefly scrolled through other code paths, and it looks like this was last major/obvious inconsistency (although code around __getattr__/__setattr__/__get__/__set__ looks a bit suspicious).

@ilevkivskyi ilevkivskyi requested a review from JukkaL Nov 27, 2019
Ivan Levkivskyi
JukkaL approved these changes Nov 27, 2019
Copy link

JukkaL left a comment

Thanks for the quick fix!

@ilevkivskyi ilevkivskyi merged commit 1122fc6 into python:master Nov 27, 2019
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
continuous-integration/travis-ci/pr The Travis CI build passed
@ilevkivskyi ilevkivskyi deleted the ilevkivskyi:freshen-classmethod branch Nov 27, 2019
ilevkivskyi added a commit that referenced this pull request Nov 29, 2019
This is a follow up for #8021, that applies the fix also to non-callable types. Currently non-callable types are still wrong:
R = TypeVar('R')
T = TypeVar('T')
# Can be any decorator that makes type non-callable.
def classproperty(f: Callable[..., R]) -> R: ...

class C(Generic[T]):
    def test(self) -> T: ...

x: C[int]
y: Type[C[int]]
reveal_type(x.test)  # Revealed type is 'int', OK
reveal_type(y.test)  # Revealed type is 'T' ???

So, #7724 strikes again. It turns out there is not only duplicated logic for attribute kinds (decorators vs normal methods), but also for callable vs non-callable types. In latter case we still need to expand the type (like in other places, e.g., `analyze_var`).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet
2 participants
You can’t perform that action at this time.