Skip to content
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.

Sign up for GitHub

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

Use Iterator instead of Iterable for specifier filter methods #584

Merged
merged 1 commit into from
Aug 17, 2022
Merged

Use Iterator instead of Iterable for specifier filter methods #584

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ Changelog
Evaluating markers like ``"extra == 'xyz'"`` without passing any extra in the
``environment`` will no longer raise an exception.
* Remove dependency on ``pyparsing``, by replacing it with a hand-written parser. This package now has no runtime dependencies (:issue:`468`)
* Update return type hint for ``Specifier.filter`` and ``SpecifierSet.filter``
to use ``Iterator`` instead of ``Iterable``

21.3 - 2021-11-17
~~~~~~~~~~~~~~~~~
Expand Down
16 changes: 8 additions & 8 deletions packaging/specifiers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ def contains(self, item: str, prereleases: Optional[bool] = None) -> bool:
@abc.abstractmethod
def filter(
self, iterable: Iterable[UnparsedVersion], prereleases: Optional[bool] = None
) -> Iterable[UnparsedVersion]:
) -> Iterator[UnparsedVersion]:
"""
Takes an iterable of items and filters them so that only items which
are contained within this specifier are allowed in it.
Expand Down Expand Up @@ -564,14 +564,14 @@ def contains(

def filter(
self, iterable: Iterable[UnparsedVersion], prereleases: Optional[bool] = None
) -> Iterable[UnparsedVersion]:
) -> Iterator[UnparsedVersion]:
"""Filter items in the given iterable, that match the specifier.

:param iterable:
An iterable that can contain version strings and :class:`Version` instances.
The items in the iterable will be filtered according to the specifier.
:param prereleases:
Whether or not to allow prereleases in the returned iterable. If set to
Whether or not to allow prereleases in the returned iterator. If set to
``None`` (the default), it will be intelligently decide whether to allow
prereleases or not (based on the :attr:`prereleases` attribute, and
whether the only versions matching are prereleases).
Expand Down Expand Up @@ -914,14 +914,14 @@ def contains(

def filter(
self, iterable: Iterable[UnparsedVersion], prereleases: Optional[bool] = None
) -> Iterable[UnparsedVersion]:
) -> Iterator[UnparsedVersion]:
"""Filter items in the given iterable, that match the specifiers in this set.

:param iterable:
An iterable that can contain version strings and :class:`Version` instances.
The items in the iterable will be filtered according to the specifier.
:param prereleases:
Whether or not to allow prereleases in the returned iterable. If set to
Whether or not to allow prereleases in the returned iterator. If set to
``None`` (the default), it will be intelligently decide whether to allow
prereleases or not (based on the :attr:`prereleases` attribute, and
whether the only versions matching are prereleases).
Expand Down Expand Up @@ -965,7 +965,7 @@ def filter(
if self._specs:
for spec in self._specs:
iterable = spec.filter(iterable, prereleases=bool(prereleases))
Copy link
Member

@brettcannon brettcannon Aug 16, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

This loop looks weird. Is it essentially running reduce on itself?

prereleases = bool(prereleases)
reduce_iterable = lambda it, spec: spec.filter(it, prereleases=prereleases)
yield from functools.reduce(reduce_iterable, self._specs, iterable)

Not sure if it's really that much more readable, though. 😅

return iterable
return iter(iterable)
Copy link
Member

Choose a reason for hiding this comment

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

Could also do:

Suggested change
return iter(iterable)
yield from iterable

Works in the other cases as well.

# If we do not have any specifiers, then we need to have a rough filter
# which will filter out any pre-releases, unless there are no final
# releases.
Expand All @@ -987,6 +987,6 @@ def filter(
# If we've found no items except for pre-releases, then we'll go
# ahead and use the pre-releases
if not filtered and found_prereleases and prereleases is None:
return found_prereleases
return iter(found_prereleases)

return filtered
return iter(filtered)