Skip to content

Conversation

@AlexWaygood
Copy link
Collaborator

@AlexWaygood AlexWaygood commented Apr 15, 2022

Fixes #205.

I've temporarily renamed the check from Y026 to Y038 so that I can see the impact using typeshed_primer in CI. I'll revert the renaming once the CI has completed.

(EDIT: The output looked good, so I reverted the renaming.)

@AlexWaygood
Copy link
Collaborator Author

I've done some random skimming of the output from typeshed_primer here, and it looks like this gets rid of the vast majority of the false positives while retaining the vast majority of the true positives.

@AlexWaygood
Copy link
Collaborator Author

There are still some false positives, even with this patch. I'm working on it.

@AlexWaygood AlexWaygood marked this pull request as draft April 15, 2022 23:38
Copy link
Collaborator

@JelleZijlstra JelleZijlstra left a comment

Choose a reason for hiding this comment

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

I know it's a draft, but I want to clear my requested review list :)

I'm wary of a heuristic-heavy approach like this since if it goes wrong, it will be hard for new contributors to understand the problem.

What if we limit ourselves to extremely obvious cases like those where the RHS has a | at the top level?

@AlexWaygood
Copy link
Collaborator Author

I know it's a draft, but I want to clear my requested review list :)

I'm wary of a heuristic-heavy approach like this since if it goes wrong, it will be hard for new contributors to understand the problem.

What if we limit ourselves to extremely obvious cases like those where the RHS has a | at the top level?

I'm moving in that direction as well after discovering more false positives when working on python/typeshed#7630 :)

@AlexWaygood
Copy link
Collaborator Author

AlexWaygood commented Apr 16, 2022

With the latest version of this patch (and following python/typeshed#7630), there are now only 4 hits in typeshed:

./stdlib/builtins.pyi:1265:1: Y026 Use typing_extensions.TypeAlias for type aliases
./stdlib/builtins.pyi:1384:1: Y026 Use typing_extensions.TypeAlias for type aliases
./stdlib/typing.pyi:1141:5: Y026 Use typing_extensions.TypeAlias for type aliases
./stdlib/typing.pyi:1153:5: Y026 Use typing_extensions.TypeAlias for type aliases

These are all places where we can't currently use TypeAlias due to some kind of bad interaction between PEP 604 and PEP 612 for mypy.

@AlexWaygood AlexWaygood marked this pull request as ready for review April 16, 2022 09:13
@srittau srittau merged commit cc09ec1 into PyCQA:master Apr 16, 2022
@AlexWaygood AlexWaygood deleted the issue-205 branch April 16, 2022 22:20
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.

Y026 TypeAlias check causes many false positives

3 participants