Skip to content

Selftype with TupleType #2436

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 8 commits into from
Nov 13, 2016
Merged

Conversation

elazarg
Copy link
Contributor

@elazarg elazarg commented Nov 11, 2016

(Reopening #2408 after the revert #2414. This fixes parts of #2090)
This PR does three things:

  1. Fix the handling of TupleType (pass original_type recursively)
  2. Fix the handling of TypeType (avoid ad-hoc buggy special casing)
  3. Make NamedTuple's _replace() and _make() return selftype, serving as test case for (1) and (2)

As a consequence of (1), some error messages are changed, as exemplified in check-isinstance.test. I think it's better now, but if it isn't, perhaps we should separate original_type and report_type as discussed in #2193.

@elazarg
Copy link
Contributor Author

elazarg commented Nov 11, 2016

So the real bug was not actually in #2408, but in my original implementation of selftype.
@gvanrossum could you test this against your internal repos?

@gvanrossum
Copy link
Member

Tested, no problems. I'll come back later with a review.

Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

Just one question.

@@ -32,9 +32,9 @@ def analyze_member_access(name: str,
is_operator: bool,
builtin_type: Callable[[str], Instance],
not_ready_callback: Callable[[str, Context], None],
msg: MessageBuilder,
msg: MessageBuilder, *,
original_type: Type,
Copy link
Member

Choose a reason for hiding this comment

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

Now that you've made this mandatory and passed it explicitly everywhere, do you still need line 53 below? (original_type = original_type or typ -- I can't put a comment there.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, I was about to remove it

@@ -391,7 +391,7 @@ def f(x: Union[List[int], List[str], int]) -> None:
else:
x[0] # E: Value of type "int" is not indexable
x + 1
x[0] # E: Value of type "int" is not indexable
x[0] # E: Value of type "Union[List[int], List[str], int]" is not indexable
Copy link
Member

Choose a reason for hiding this comment

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

I'm guessing this is because of the better determination of original_type right? I approve of the new error message.

@elazarg
Copy link
Contributor Author

elazarg commented Nov 13, 2016

(The last commit also makes some cosmetic changes)

@gvanrossum gvanrossum merged commit f178a6c into python:master Nov 13, 2016
@gvanrossum
Copy link
Member

Thanks! This is a real improvement.

@gvanrossum
Copy link
Member

BTW Can you write up some docs for self-type? I think it would be nice if we had some examples for this in mypy's chapter on generics.

@JukkaL
Copy link
Collaborator

JukkaL commented Nov 14, 2016

We are probably going to have a mypy release this week. It would be good to have the docs so that we can ask users to give it a spin.

@elazarg elazarg deleted the namedtuple_selftype branch November 14, 2016 15:07
@elazarg
Copy link
Contributor Author

elazarg commented Nov 14, 2016

#2450. I am not confident with the examples, the phrasing, or the English grammar :|

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.

3 participants