Skip to content

Improve None as predicate for Python 2 ifilter - #8191

Merged
srittau merged 2 commits into
python:masterfrom
david-askari:master
Jun 28, 2022
Merged

srittau merged 2 commits into
python:masterfrom
david-askari:master

Conversation

@david-askari

Copy link
Copy Markdown
Contributor

Properly support ifilter(None, ...) on Python 2. Used Python 3 filter as a basis.

I stumbled over this as six.moves.filter returns itertools.ifilter.

@github-actions

This comment has been minimized.

@AlexWaygood

Copy link
Copy Markdown
Member

Thanks! Unfortunately, I know very little about Python 2, so I won't be able to review this (though one of the other maintainers might).

Please be aware, however, that we're planning on deleting all of our remaining Python 2 stubs as soon as mypy drops support for Python 2, which could be very soon (#7367, python/mypy#12237)

Comment thread stdlib/@python2/itertools.pyi Outdated
@overload
def ifilter(predicate: Callable[[_T], Any], iterable: Iterable[_T]) -> Iterator[_T]: ...
@overload
def ifilterfalse(predicate: None, iterable: Iterable[_T | None]) -> Iterator[_T]: ...

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.

I don't think this is correct. Using Python 3:

>>> from itertools import filterfalse
>>> list(filterfalse(None, [0,1,2,3,None]))
[0, None]

I don't think an overload for ifilterfalse makes sense here.

@github-actions

Copy link
Copy Markdown
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

@david-askari

Copy link
Copy Markdown
Contributor Author

@AlexWaygood thanks for the heads-up, I was expecting something like this 😬

@srittau I removed the ifilterfalse overload.

@david-askari
david-askari requested a review from srittau June 28, 2022 11:38
@srittau
srittau merged commit 2ffa756 into python:master Jun 28, 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