Skip to content

[red-knot] Understanding type[Union[A, B]]#14858

Merged
AlexWaygood merged 6 commits intoastral-sh:mainfrom
InSyncWithFoo:rk-type-union
Dec 9, 2024
Merged

[red-knot] Understanding type[Union[A, B]]#14858
AlexWaygood merged 6 commits intoastral-sh:mainfrom
InSyncWithFoo:rk-type-union

Conversation

@InSyncWithFoo
Copy link
Copy Markdown
Contributor

Summary

Resolves #14834.

Test Plan

Markdown tests.

@InSyncWithFoo
Copy link
Copy Markdown
Contributor Author

The failing test boils down to this:

import collections

MyObj1 = collections.namedtuple("MyObj1", ["a", "b"])

@AlexWaygood AlexWaygood added the ty Multi-file analysis & type inference label Dec 9, 2024
Copy link
Copy Markdown
Member

@MichaReiser MichaReiser left a comment

Choose a reason for hiding this comment

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

The tests are failing with a panic in an assertion. Would you mind taking a look at what's happening

Copy link
Copy Markdown
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

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

Thanks. There were a couple of issues with your initial patch; I've pushed to your branch to fix them:

  • You didn't handle single-element unions such as type[Union[int]] (the subscript slice inside the Union[] is not an Expr::Tuple if there's only a single element).
  • You were failing to store the types of subexpressions for subscripts that are either invalid or not-yet-supported; that was causing panics in the corpus test. It can be surprisingly tricky to make sure that you infer the type of each subexpression exactly once; it took me longer than I expected to fix this!
  • Going forward, we'd prefer to use parameter annotations in mdtests to test type expressions rather than return annotations, as they're more concise (#14839)

@AlexWaygood AlexWaygood dismissed MichaReiser’s stale review December 9, 2024 12:36

it doesn't panic anymore

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Dec 9, 2024

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@AlexWaygood AlexWaygood merged commit 3865fb6 into astral-sh:main Dec 9, 2024
@InSyncWithFoo InSyncWithFoo deleted the rk-type-union branch December 9, 2024 12:49
@InSyncWithFoo
Copy link
Copy Markdown
Contributor Author

@AlexWaygood Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ty Multi-file analysis & type inference

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[red-knot] Understand type[Union[A, B]] where A and B are both classes

3 participants