Add CandidateEvaluator class to encapsulate sorting. (#6424)

Author: cjerdonek

src/pip/_internal/commands/list.py

@@ -182,10 +182,11 @@ def iter_packages_latest_infos(self, packages, options):
                     all_candidates = [candidate for candidate in all_candidates
                                       if not candidate.version.is_prerelease]
 
-                if not all_candidates:
+                evaluator = finder.candidate_evaluator
+                best_candidate = evaluator.get_best_candidate(all_candidates)
+                if best_candidate is None:
                     continue
-                best_candidate = max(all_candidates,
-                                     key=finder._candidate_sort_key)
+
                 remote_version = best_candidate.version
                 if best_candidate.location.is_wheel:
                     typ = 'wheel'

src/pip/_internal/index.py

@@ -48,6 +48,7 @@
     )
     from pip._vendor.packaging.version import _BaseVersion
     from pip._vendor.requests import Response
+    from pip._internal.pep425tags import Pep425Tag
     from pip._internal.req import InstallRequirement
     from pip._internal.download import PipSession
 
@@ -255,6 +256,71 @@ def _get_html_page(link, session=None):
     return None
 
 
+class CandidateEvaluator(object):
+
+    def __init__(
+        self,
+        valid_tags,          # type: List[Pep425Tag]
+        prefer_binary=False  # type: bool
+
+    ):
+        # type: (...) -> None
+        self._prefer_binary = prefer_binary
+        self._valid_tags = valid_tags
+
+    def is_wheel_supported(self, wheel):
+        # type: (Wheel) -> bool
+        return wheel.supported(self._valid_tags)
+
+    def _sort_key(self, candidate):
+        # type: (InstallationCandidate) -> CandidateSortingKey
+        """
+        Function used to generate link sort key for link tuples.
+        The greater the return value, the more preferred it is.
+        If not finding wheels, then sorted by version only.
+        If finding wheels, then the sort order is by version, then:
+          1. existing installs
+          2. wheels ordered via Wheel.support_index_min(self._valid_tags)
+          3. source archives
+        If prefer_binary was set, then all wheels are sorted above sources.
+        Note: it was considered to embed this logic into the Link
+              comparison operators, but then different sdist links
+              with the same version, would have to be considered equal
+        """
+        support_num = len(self._valid_tags)
+        build_tag = tuple()  # type: BuildTag
+        binary_preference = 0
+        if candidate.location.is_wheel:
+            # can raise InvalidWheelFilename
+            wheel = Wheel(candidate.location.filename)
+            if not wheel.supported(self._valid_tags):
+                raise UnsupportedWheel(
+                    "%s is not a supported wheel for this platform. It "
+                    "can't be sorted." % wheel.filename
+                )
+            if self._prefer_binary:
+                binary_preference = 1
+            pri = -(wheel.support_index_min(self._valid_tags))
+            if wheel.build_tag is not None:
+                match = re.match(r'^(\d+)(.*)$', wheel.build_tag)
+                build_tag_groups = match.groups()
+                build_tag = (int(build_tag_groups[0]), build_tag_groups[1])
+        else:  # sdist
+            pri = -(support_num)
+        return (binary_preference, candidate.version, build_tag, pri)
+
+    def get_best_candidate(self, candidates):
+        # type: (List[InstallationCandidate]) -> InstallationCandidate
+        """
+        Return the best candidate per the instance's sort order, or None if
+        no candidates are given.
+        """
+        if not candidates:
+            return None
+
+        return max(candidates, key=self._sort_key)
+
+
 class FoundCandidates(object):
     """A collection of candidates, returned by `PackageFinder.find_candidates`.
 
@@ -267,18 +333,18 @@ class FoundCandidates(object):
     * `specifier`: Specifier to filter applicable versions.
     * `prereleases`: Whether prereleases should be accounted. Pass None to
         infer from the specifier.
-    * `sort_key`: A callable used as the key function when choosing the best
-        candidate.
+    * `evaluator`: A CandidateEvaluator object to sort applicable candidates
+        by order of preference.
     """
     def __init__(
         self,
         candidates,     # type: List[InstallationCandidate]
         versions,       # type: Set[str]
-        sort_key,       # type: Callable[[InstallationCandidate], Any]
+        evaluator,      # type: CandidateEvaluator
     ):
         # type: (...) -> None
         self._candidates = candidates
-        self._sort_key = sort_key
+        self._evaluator = evaluator
         self._versions = versions
 
     @classmethod
@@ -287,7 +353,7 @@ def from_specifier(
         candidates,     # type: List[InstallationCandidate]
         specifier,      # type: specifiers.BaseSpecifier
         prereleases,    # type: Optional[bool]
-        sort_key,       # type: Callable[[InstallationCandidate], Any]
+        evaluator,      # type: CandidateEvaluator
     ):
         # type: (...) -> FoundCandidates
         versions = {
@@ -303,7 +369,7 @@ def from_specifier(
                 prereleases=prereleases,
             )
         }
-        return cls(candidates, versions, sort_key)
+        return cls(candidates, versions, evaluator)
 
     def iter_all(self):
         # type: () -> Iterable[InstallationCandidate]
@@ -325,9 +391,7 @@ def get_best(self):
         candidates are found.
         """
         candidates = list(self.iter_applicable())
-        if not candidates:
-            return None
-        return max(candidates, key=self._sort_key)
+        return self._evaluator.get_best_candidate(candidates)
 
 
 class PackageFinder(object):
@@ -368,6 +432,8 @@ def __init__(
             to pep425tags.py in the get_supported() method.
         :param implementation: A string or None. This is passed directly
             to pep425tags.py in the get_supported() method.
+        :param prefer_binary: Whether to prefer an old, but valid, binary
+            dist over a new source dist.
         """
         if session is None:
             raise TypeError(
@@ -408,15 +474,15 @@ def __init__(
         self.session = session
 
         # The valid tags to check potential found wheel candidates against
-        self.valid_tags = get_supported(
+        valid_tags = get_supported(
             versions=versions,
             platform=platform,
             abi=abi,
             impl=implementation,
         )
-
-        # Do we prefer old, but valid, binary dist over new source dist
-        self.prefer_binary = prefer_binary
+        self.candidate_evaluator = CandidateEvaluator(
+            valid_tags=valid_tags, prefer_binary=prefer_binary,
+        )
 
         # If we don't have TLS enabled, then WARN if anyplace we're looking
         # relies on TLS.
@@ -503,43 +569,6 @@ def sort_path(path):
 
         return files, urls
 
-    def _candidate_sort_key(self, candidate):
-        # type: (InstallationCandidate) -> CandidateSortingKey
-        """
-        Function used to generate link sort key for link tuples.
-        The greater the return value, the more preferred it is.
-        If not finding wheels, then sorted by version only.
-        If finding wheels, then the sort order is by version, then:
-          1. existing installs
-          2. wheels ordered via Wheel.support_index_min(self.valid_tags)
-          3. source archives
-        If prefer_binary was set, then all wheels are sorted above sources.
-        Note: it was considered to embed this logic into the Link
-              comparison operators, but then different sdist links
-              with the same version, would have to be considered equal
-        """
-        support_num = len(self.valid_tags)
-        build_tag = tuple()  # type: BuildTag
-        binary_preference = 0
-        if candidate.location.is_wheel:
-            # can raise InvalidWheelFilename
-            wheel = Wheel(candidate.location.filename)
-            if not wheel.supported(self.valid_tags):
-                raise UnsupportedWheel(
-                    "%s is not a supported wheel for this platform. It "
-                    "can't be sorted." % wheel.filename
-                )
-            if self.prefer_binary:
-                binary_preference = 1
-            pri = -(wheel.support_index_min(self.valid_tags))
-            if wheel.build_tag is not None:
-                match = re.match(r'^(\d+)(.*)$', wheel.build_tag)
-                build_tag_groups = match.groups()
-                build_tag = (int(build_tag_groups[0]), build_tag_groups[1])
-        else:  # sdist
-            pri = -(support_num)
-        return (binary_preference, candidate.version, build_tag, pri)
-
     def _validate_secure_origin(self, logger, location):
         # type: (Logger, Link) -> bool
         # Determine if this url used a secure transport mechanism
@@ -722,7 +751,7 @@ def find_candidates(
             self.find_all_candidates(project_name),
             specifier=specifier,
             prereleases=(self.allow_all_prereleases or None),
-            sort_key=self._candidate_sort_key,
+            evaluator=self.candidate_evaluator,
         )
 
     def find_requirement(self, req, upgrade):
@@ -893,7 +922,7 @@ def _link_package_versions(self, link, search):
                         link, 'wrong project name (not %s)' % search.supplied)
                     return None
 
-                if not wheel.supported(self.valid_tags):
+                if not self.candidate_evaluator.is_wheel_supported(wheel):
                     self._log_skipped_link(
                         link, 'it is not compatible with this Python')
                     return None

tests/unit/test_finder.py

@@ -12,7 +12,7 @@
     BestVersionAlreadyInstalled, DistributionNotFound,
 )
 from pip._internal.index import (
-    InstallationCandidate, Link, PackageFinder, Search,
+    CandidateEvaluator, InstallationCandidate, Link, PackageFinder, Search,
 )
 from pip._internal.req.constructors import install_req_from_line
 
@@ -154,7 +154,8 @@ def test_not_find_wheel_not_supported(self, data, monkeypatch):
             [],
             session=PipSession(),
         )
-        finder.valid_tags = pip._internal.pep425tags.get_supported()
+        valid_tags = pip._internal.pep425tags.get_supported()
+        finder.candidate_evaluator = CandidateEvaluator(valid_tags=valid_tags)
 
         with pytest.raises(DistributionNotFound):
             finder.find_requirement(req, True)
@@ -243,16 +244,15 @@ def test_link_sorting(self):
                 Link('simple-1.0.tar.gz'),
             ),
         ]
-        finder = PackageFinder([], [], session=PipSession())
-        finder.valid_tags = [
+        valid_tags = [
             ('pyT', 'none', 'TEST'),
             ('pyT', 'TEST', 'any'),
             ('pyT', 'none', 'any'),
         ]
-        results = sorted(links,
-                         key=finder._candidate_sort_key, reverse=True)
-        results2 = sorted(reversed(links),
-                          key=finder._candidate_sort_key, reverse=True)
+        evaluator = CandidateEvaluator(valid_tags=valid_tags)
+        sort_key = evaluator._sort_key
+        results = sorted(links, key=sort_key, reverse=True)
+        results2 = sorted(reversed(links), key=sort_key, reverse=True)
 
         assert links == results == results2, results2
 
@@ -276,9 +276,9 @@ def test_link_sorting_wheels_with_build_tags(self):
             ),
         ]
         finder = PackageFinder([], [], session=PipSession())
-        results = sorted(links, key=finder._candidate_sort_key, reverse=True)
-        results2 = sorted(reversed(links), key=finder._candidate_sort_key,
-                          reverse=True)
+        sort_key = finder.candidate_evaluator._sort_key
+        results = sorted(links, key=sort_key, reverse=True)
+        results2 = sorted(reversed(links), key=sort_key, reverse=True)
         assert links == results == results2, results2