Skip to content

Replace Union annotations with TypeVar #26453

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

Closed
WillAyd opened this issue May 19, 2019 · 6 comments · Fixed by #26588
Closed

Replace Union annotations with TypeVar #26453

WillAyd opened this issue May 19, 2019 · 6 comments · Fixed by #26588
Labels
Typing type annotations, mypy/pyright type checking
Milestone

Comments

@WillAyd
Copy link
Member

WillAyd commented May 19, 2019

Had a revelation after python/mypy#6847 (comment) and I think we've been using Union too liberally in places where a TypeVar may make more sense (especially in pandas._typing) so would be good to go back and swap those out.

@gwrome @vaibhavhrt

@WillAyd WillAyd added the Typing type annotations, mypy/pyright type checking label May 19, 2019
@WillAyd WillAyd added this to the Contributions Welcome milestone May 19, 2019
@vaibhavhrt
Copy link
Contributor

@WillAyd we are almost done with removing all ignored modules from mypy.ini Once that's done I will take a look where we have used Union and see if we can use TypeVar instead.

@vaibhavhrt
Copy link
Contributor

vaibhavhrt commented May 29, 2019

@WillAyd I will look for Union in pandas._typing and after fixing that will look for Union in other files.
Just to be sure typeVar for Union[str, float] would be Something = TypeVar('Something', str, float), right?
Also can please provide some explanation on when to use TypeVar and when to use Union.

@WillAyd
Copy link
Member Author

WillAyd commented May 29, 2019

TypeVar is going to be more suitable for parametrization and generic functions. In the case of containers I think the difference between:

T = TypeVar('T', str, float)
List[T]

and

List[Union[str, float]]

Would be that the former could only accept [1, 2] and ['a', 'b'] but not [1, 'a']. The latter would accept all of those.

Looking at functions:

def foo(something: T) -> T:
    ...

Would basically mean that if foo accepts a string it also returns a string, and if it accepts a float it returns a float, but that it cannot accept a string and return a float. By contrast:

def foo(Union[str, float]) -> Union[str, float]:
    ...

Would allow all of those combinations, so it could accept 'a' and return 1.

We've used Union throughout so far but more often than not I think parametrization of the Type is actually what we want.

@vaibhavhrt
Copy link
Contributor

Nice, thanks for explaining, I will replace all unions in pandas._typing today.

@jreback jreback modified the milestones: Contributions Welcome, 0.25.0 Jun 1, 2019
@jreback
Copy link
Contributor

jreback commented Jun 8, 2019

does #26588 fully close this?

@WillAyd
Copy link
Member Author

WillAyd commented Jun 9, 2019

Yep

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Typing type annotations, mypy/pyright type checking
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants