Skip to content

Added Optional[] around key argument to itertools.groupby() - #2790

Merged
srittau merged 1 commit into
python:masterfrom
achampion:itertools-groupby-optional
Feb 12, 2019
Merged

srittau merged 1 commit into
python:masterfrom
achampion:itertools-groupby-optional

Conversation

@achampion

Copy link
Copy Markdown
Contributor

See #2788

The simplest case of:

import itertools
iterator = itertools.groupby(range(10), None)

gives the type error: error: No overload variant of "groupby" matches argument types "range", "None"

iterator = itertools.groupby(range(10))

has no error.
This example is a little contrived though legal, the case came up in the context of a Optional[] argument to function, e.g.:

T = TypeVar('T')
S = TypeVar('S')

def fn(iterable: Iterable, key: Optional[Callable[[T], S]] = None) -> Iterator[Tuple[S, Iterator[T]]]:
    iterator: Iterator[Tuple[S, Iterator[T]]] = itertools.groupby(iterable, key)
    return iterator

error: Argument 2 to "groupby" has incompatible type "Optional[Callable[[T], S]]"; expected "Callable[[T], S]"

…o allow `None` to pass the type

checker.

@srittau srittau left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks!

@srittau
srittau merged commit 9b545a0 into python:master Feb 12, 2019
@achampion
achampion deleted the itertools-groupby-optional branch February 12, 2019 19:05
Comment thread stdlib/2/itertools.pyi
@overload
def groupby(iterable: Iterable[_T],
key: Callable[[_T], _S]) -> Iterator[Tuple[_S, Iterator[_T]]]: ...
key: Optional[Callable[[_T], _S]]) -> Iterator[Tuple[_S, Iterator[_T]]]: ...

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Shouldn't we add a separate overload instead? If key is None, there's nothing to constrain the value of _S.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@JelleZijlstra Good point, opened #2794.

srittau added a commit to srittau/typeshed that referenced this pull request Feb 13, 2019
JelleZijlstra pushed a commit that referenced this pull request Feb 13, 2019
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