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

tuple slice should not propagate fallback #16154

Merged
merged 6 commits into from Oct 1, 2023

Conversation

graingert
Copy link
Contributor

@graingert graingert commented Sep 21, 2023

Fixes #8776

NamedTuple (or any other tuple subtype) slicing should not propagate the fallback

@graingert
Copy link
Contributor Author

I want to add #8776 (comment) as a test case, but I'm not sure where

@@ -2416,13 +2416,14 @@ def copy_modified(
items = self.items
return TupleType(items, fallback, self.line, self.column)

def slice(self, begin: int | None, end: int | None, stride: int | None) -> TupleType:
def slice(
self, begin: int | None, end: int | None, stride: int | None, *, fallback: Instance | None
Copy link
Contributor Author

@graingert graingert Sep 21, 2023

Choose a reason for hiding this comment

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

fallback should always be named_type("builtins.tuple") but I'm not sure how to make one without access to a TypeChecker instance

@graingert graingert marked this pull request as ready for review September 21, 2023 13:47
@github-actions

This comment has been minimized.

Copy link
Collaborator

@hauntsaninja hauntsaninja left a comment

Choose a reason for hiding this comment

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

Thanks, merge conflicts aside, this looks good to me. test-data/unit/check-tuples.test is probably a good place for the test you suggest.

@graingert
Copy link
Contributor Author

@hauntsaninja ok conflicts resolved

@github-actions

This comment has been minimized.

@hauntsaninja
Copy link
Collaborator

Tests are failing, let me know if you could use assistance on getting your nice #8776 (comment) example in a test

mypy/types.py Outdated Show resolved Hide resolved
@github-actions

This comment has been minimized.

@graingert
Copy link
Contributor Author

@hauntsaninja ok I added that test

@github-actions
Copy link
Contributor

github-actions bot commented Oct 1, 2023

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

Copy link
Collaborator

@hauntsaninja hauntsaninja left a comment

Choose a reason for hiding this comment

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

Thank you!

@hauntsaninja hauntsaninja merged commit 99ba048 into python:master Oct 1, 2023
18 checks passed
@graingert graingert deleted the tuple-fallback-fix branch October 1, 2023 21:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect Error with NamedTuple slices ?
2 participants