Skip to content

Conversation

@sobolevn
Copy link
Member

This is more an experiment to see if newly added TypeAlias in our own code is supported.

Refs #11305

@sobolevn
Copy link
Member Author

Looks like it does not work out of the box. Some more work is required.
@hauntsaninja, maybe you will be interested in looking into failing logs.

@sobolevn sobolevn marked this pull request as draft October 12, 2021 22:38
@hauntsaninja
Copy link
Collaborator

Thanks for doing this, will take a look later. I suspect at least some of the confusion might just be from clobbering mypy.nodes.TypeAlias

mypy/mypy/nodes.py

Lines 2877 to 2878 in c22beb4

class TypeAlias(SymbolNode):

mypy/checker.py Outdated
Iterable, Sequence, Mapping, Generic, AbstractSet, Callable
)
from typing_extensions import Final
from typing_extensions import Final, TypeAlias
Copy link
Member

Choose a reason for hiding this comment

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

This file also imports mypy.nodes.TypeAlias (line 25)

Any, cast, Dict, Set, List, Tuple, Callable, Union, Optional, Sequence, Iterator
)
from typing_extensions import ClassVar, Final, overload
from typing_extensions import ClassVar, Final, overload, TypeAlias
Copy link
Member

Choose a reason for hiding this comment

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

This one too (line 34)

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks! Now TypeAlias from typing_extensions is always called _TypeAlias

@JelleZijlstra
Copy link
Member

@hauntsaninja is right. I wonder why flake8 doesn't catch this.

@sobolevn sobolevn marked this pull request as ready for review October 13, 2021 07:30
@sobolevn sobolevn closed this Oct 13, 2021
@sobolevn sobolevn reopened this Oct 13, 2021
@sobolevn
Copy link
Member Author

Looks like the CI is stuck. Reopening to retrigger it.

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.

Hooray, looks like it's passing tests.

I'd recommend using _TypeAlias in every file, not just ones that we have mypy.nodes.TypeAlias in. Actually, thinking about it, I think the clearest thing to do is just use the fully qualified typing_extensions.TypeAlias everywhere in mypy. There aren't too many of these, so the extra verbosity is probably worth it.

I think we'll also need to change the minimum version of typing_extensions we require.

@sobolevn
Copy link
Member Author

I'd recommend using _TypeAlias in every file, not just ones that we have mypy.nodes.TypeAlias in.

I've already done this 🎉

I think the clearest thing to do is just use the fully qualified typing_extensions.TypeAlias everywhere in mypy.

I cannot agree with this, sorry! This is way too verbose. Imagine this:

import typing_extensions

TypeParameterChecker: typing_extensions.TypeAlias = Callable[[Type, Type, int], bool]

Current solution:

from typing_extensions import TypeAlias as _TypeAlias

TypeParameterChecker: _TypeAlias = Callable[[Type, Type, int], bool]

Looks and reads much better! 😎
Moreover, we won't have to duplicate typing_extensions import, in case some other names like from typing_extensions import Final are also used.

@sobolevn sobolevn closed this Oct 13, 2021
@sobolevn sobolevn reopened this Oct 13, 2021
mypy/report.py Outdated
)

ReporterClasses = Dict[str, Tuple[Callable[['Reports', str], 'AbstractReporter'], bool]]
ReporterClasses: TypeAlias = Dict[str, Tuple[Callable[['Reports', str], 'AbstractReporter'], bool]]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
ReporterClasses: TypeAlias = Dict[str, Tuple[Callable[['Reports', str], 'AbstractReporter'], bool]]
ReporterClasses: _TypeAlias = Dict[str, Tuple[Callable[['Reports', str], 'AbstractReporter'], bool]]

mypy/scope.py Outdated


SavedScope = Tuple[str, Optional[TypeInfo], Optional[FuncBase]]
SavedScope: TypeAlias = Tuple[str, Optional[TypeInfo], Optional[FuncBase]]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
SavedScope: TypeAlias = Tuple[str, Optional[TypeInfo], Optional[FuncBase]]
SavedScope: _TypeAlias = Tuple[str, Optional[TypeInfo], Optional[FuncBase]]


# Represents that the 'left' instance is a subtype of the 'right' instance
SubtypeRelationship = Tuple[Instance, Instance]
SubtypeRelationship: TypeAlias = Tuple[Instance, Instance]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
SubtypeRelationship: TypeAlias = Tuple[Instance, Instance]
SubtypeRelationship: _TypeAlias = Tuple[Instance, Instance]

# A tuple encoding the specific conditions under which we performed the subtype check.
# (e.g. did we want a proper subtype? A regular subtype while ignoring variance?)
SubtypeKind = Tuple[bool, ...]
SubtypeKind: TypeAlias = Tuple[bool, ...]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
SubtypeKind: TypeAlias = Tuple[bool, ...]
SubtypeKind: _TypeAlias = Tuple[bool, ...]

# A cache that keeps track of whether the given TypeInfo is a part of a particular
# subtype relationship
SubtypeCache = Dict[TypeInfo, Dict[SubtypeKind, Set[SubtypeRelationship]]]
SubtypeCache: TypeAlias = Dict[TypeInfo, Dict[SubtypeKind, Set[SubtypeRelationship]]]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
SubtypeCache: TypeAlias = Dict[TypeInfo, Dict[SubtypeKind, Set[SubtypeRelationship]]]
SubtypeCache: _TypeAlias = Dict[TypeInfo, Dict[SubtypeKind, Set[SubtypeRelationship]]]

@JelleZijlstra
Copy link
Member

What if we rename mypy.nodes.TypeAlias instead?

@hauntsaninja
Copy link
Collaborator

hauntsaninja commented Oct 13, 2021

We already have both a TypeAlias and a TypeAliasType + might be a breaking change for plugins. I'm open to it, but I'm struggling to come up with names that work. Maybe we should just rewrite all of mypy in TypeScript? ;-)
I think I still like the verbose option the best, but don't feel strongly enough about it to push for it since Nikita disagrees.

@sobolevn
Copy link
Member Author

sobolevn commented Oct 13, 2021

Thanks! I've missed a couple. Now it should be ready!

Maybe we should just rewrite all of mypy in TypeScript? ;-)

I am planning to do it in Rust 😉

@hauntsaninja hauntsaninja merged commit f497645 into python:master Oct 14, 2021
@hauntsaninja
Copy link
Collaborator

Thank you!

tushar-deepsource pushed a commit to DeepSourceCorp/mypy that referenced this pull request Jan 20, 2022
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