Skip to content

builtins.pyi: Use two type vars #3291

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

Merged
merged 2 commits into from
Oct 3, 2019
Merged

builtins.pyi: Use two type vars #3291

merged 2 commits into from
Oct 3, 2019

Conversation

utkarsh2102
Copy link
Contributor

Fixes: #3201

Copy link
Collaborator

@srittau srittau left a comment

Choose a reason for hiding this comment

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

Unfortunately this does not work, at least with mypy. The best we can probably do here is to use a liberal sprinkling of Any. Testing also revealed that special-casing Optional works with the example provided in #3201. Since None is most likely often used as sentinel, special casing it makes sense to me:

@overload
def iter(__function: Callable[[], Optional[_T]], __sentinel: None) -> Iterator[_T]: ...
@overload
def iter(__function: Callable[[], Any], __sentinel: Any) -> Iterator[Any]: ...

cc @JelleZijlstra for another pair of eyes.

@utkarsh2102
Copy link
Contributor Author

The best we can probably do here is to use a liberal sprinkling of Any.

Ack, makes sense. Thank you!

@srittau srittau merged commit fa571fb into python:master Oct 3, 2019
@wbolster-eiq
Copy link

thanks all, but it seems to me the fallback @overload is now less strict than it was before?

@overload
def iter(__function: Callable[[], Any], __sentinel: Any) -> Iterator[Any]: ...

the original was:

def iter(__function: Callable[[], _T], __sentinel: _T) -> Iterator[_T]: ...

...which seems more strict and hence more correct?

i am curious to learn why adding a special case for the (indeed common) None sentinel is not sufficient?

@srittau
Copy link
Collaborator

srittau commented Oct 7, 2019

@wbolster-eiq Without the overload, the following valid case is rejected:

def foo() -> Union[str, int]:
    return ""
reveal_type(iter(foo, -1))

That said, keeping the other overload as second-to-last makes sense to keep the more precise typing for the common case. Would you care to send a PR?

@wbolster-eiq
Copy link

@srittau do i understand correctly that the complete definition should be like this?

@overload
def iter(__iterable: Iterable[_T]) -> Iterator[_T]: ...

@overload
def iter(__function: Callable[[], Optional[_T]], __sentinel: None) -> Iterator[_T]: ...

@overload
def iter(__function: Callable[[], _T], __sentinel: _T) -> Iterator[_T]: ...

if so, yeah, i can turn that into a pull request, but right now i'm not sure i understand 100%.

@srittau
Copy link
Collaborator

srittau commented Oct 7, 2019

@wbolster-eiq: We would still need the Any overload as last one to allow the example I gave. But correct apart from that.

def iter(__function: Callable[[], _T], __sentinel: _T) -> Iterator[_T]: ...
def iter(__function: Callable[[], Optional[_T]], __sentinel: None) -> Iterator[_T]: ...
@overload
def iter(__function: Callable[[], Any], __sentinel: Any) -> Iterator[Any]: ...

Choose a reason for hiding this comment

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

the Any is too relaxed here. the output from the callable will always be the same as the value yielded by the iterator, regardless of what the sentinel type is. i will open a pull request to make this more strict.

Copy link
Contributor

Choose a reason for hiding this comment

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

@wbolster
Copy link
Contributor

wbolster commented Oct 8, 2019

i've opened #3326 as a follow-up to this pull request.

srittau pushed a commit that referenced this pull request Oct 9, 2019
)

This is a continuation of #3291, which was the initial fix for #3201.

The 2-arg version of iter() turns a callable into an iterator. The
changes made in #3291 introduce an Any return type for both the
callable's return type and the iterator's type, while in reality the
return type of the function is always the same as the iterator's type.
@utkarsh2102 utkarsh2102 deleted the iter branch October 9, 2019 20:25
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.

two-arg iter() with None sentinel does not remove ‘Optional’
4 participants