NotificationsYou must be signed in to change notification settings
Fork3.1k
Star20.1k

Check tuples of abstract types#15366

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

sobolevn merged 5 commits intomasterfromissue-15264

Jul 12, 2023

Merged

Check tuples of abstract types#15366

sobolevn merged 5 commits intomasterfromissue-15264

Jul 12, 2023

Conversation

Copy link

Member

sobolevn commentedJun 4, 2023

The PR is quite simple (and I would like to keep it this way):

Before we were only checkingtype[] type
Now we also checktuple[type[], ...] type

There might be other types that we want to add in the future here:TypedDictType, etc?
But, let's do it one by one, because smaller PRs are easier to merge :)

Closes#15264

sobolevn added3 commits

June 4, 2023 10:39

Check tuples of abstract types, refs#15264

88485fd

Fix CI

41f4dd7

Fix CI

74984e1

This comment has been minimized.

Fix CI

650c979

This comment has been minimized.

sobolevn requested a review fromhauntsaninja

June 4, 2023 11:05

Copy link

Contributor

First impression: shouldn't there be something else that recurses into a tuple already to validate types of items, so we could avoid teachinghas_abstract_type_part aboutTupleType (and then aboutTypedDictType etc.) ?

Copy link

MemberAuthor

sobolevn commentedJun 12, 2023

I don't see a clean way to do that.

Copy link

Collaborator

JukkaL commentedJul 12, 2023

I don't see a clean way to do that.

What about just leaving a TODO comment about this, in case somebody figures out a good way to do that?

JukkaL approved these changes

Jul 12, 2023

View reviewed changes

Copy link

Collaborator

JukkaL left a comment

Choose a reason for hiding this comment

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

Even if this doesn't fully solve the issue, this seems like a reasonably incremental improvement, though it would be good to add a comment explaining that this is incomplete (and perhaps why), as I mentioned in my other comment.