Skip to content

Remove lru cache from methods [enable ruff rule cached-instance-method (B019)] #13306

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 5 commits into from
Apr 21, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 0 additions & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,6 @@ extend-exclude = [

[tool.ruff.lint]
ignore = [
"B019",
"B020",
"B904", # Ruff enables opinionated warnings by default
"B905", # Ruff enables opinionated warnings by default
Expand Down
42 changes: 36 additions & 6 deletions src/pip/_internal/index/package_finder.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,17 @@
import logging
import re
from dataclasses import dataclass
from typing import TYPE_CHECKING, FrozenSet, Iterable, List, Optional, Set, Tuple, Union
from typing import (
TYPE_CHECKING,
Dict,
FrozenSet,
Iterable,
List,
Optional,
Set,
Tuple,
Union,
)

from pip._vendor.packaging import specifiers
from pip._vendor.packaging.tags import Tag
Expand Down Expand Up @@ -601,6 +611,13 @@ def __init__(
# These are boring links that have already been logged somehow.
self._logged_links: Set[Tuple[Link, LinkType, str]] = set()

# Cache of the result of finding candidates
self._all_candidates: Dict[str, List[InstallationCandidate]] = {}
self._best_candidates: Dict[
Tuple[str, Optional[specifiers.BaseSpecifier], Optional[Hashes]],
BestCandidateResult,
] = {}

# Don't include an allow_yanked default value to make sure each call
# site considers whether yanked releases are allowed. This also causes
# that decision to be made explicit in the calling code, which helps
Expand Down Expand Up @@ -800,7 +817,6 @@ def process_project_url(

return package_links

@functools.lru_cache(maxsize=None)
def find_all_candidates(self, project_name: str) -> List[InstallationCandidate]:
"""Find all available InstallationCandidate for project_name

Expand All @@ -810,6 +826,9 @@ def find_all_candidates(self, project_name: str) -> List[InstallationCandidate]:
See LinkEvaluator.evaluate_link() for details on which files
are accepted.
"""
if project_name in self._all_candidates:
return self._all_candidates[project_name]

link_evaluator = self.make_link_evaluator(project_name)

collected_sources = self._link_collector.collect_sources(
Expand Down Expand Up @@ -851,7 +870,9 @@ def find_all_candidates(self, project_name: str) -> List[InstallationCandidate]:
logger.debug("Local files found: %s", ", ".join(paths))

# This is an intentional priority ordering
return file_candidates + page_candidates
self._all_candidates[project_name] = file_candidates + page_candidates

return self._all_candidates[project_name]

def make_candidate_evaluator(
self,
Expand All @@ -870,7 +891,6 @@ def make_candidate_evaluator(
hashes=hashes,
)

@functools.lru_cache(maxsize=None)
def find_best_candidate(
self,
project_name: str,
Expand All @@ -885,13 +905,20 @@ def find_best_candidate(

:return: A `BestCandidateResult` instance.
"""
if (project_name, specifier, hashes) in self._best_candidates:
return self._best_candidates[project_name, specifier, hashes]

candidates = self.find_all_candidates(project_name)
candidate_evaluator = self.make_candidate_evaluator(
project_name=project_name,
specifier=specifier,
hashes=hashes,
)
return candidate_evaluator.compute_best_candidate(candidates)
self._best_candidates[project_name, specifier, hashes] = (
candidate_evaluator.compute_best_candidate(candidates)
)

return self._best_candidates[project_name, specifier, hashes]

def find_requirement(
self, req: InstallRequirement, upgrade: bool
Expand All @@ -902,9 +929,12 @@ def find_requirement(
Returns a InstallationCandidate if found,
Raises DistributionNotFound or BestVersionAlreadyInstalled otherwise
"""
name = req.name
assert name is not None, "find_requirement() called with no name"

hashes = req.hashes(trust_internet=False)
best_candidate_result = self.find_best_candidate(
req.name,
name,
specifier=req.specifier,
hashes=hashes,
)
Expand Down
11 changes: 8 additions & 3 deletions src/pip/_internal/resolution/resolvelib/found_candidates.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
something.
"""

import functools
import logging
from collections.abc import Sequence
from typing import Any, Callable, Iterator, Optional, Set, Tuple
Expand Down Expand Up @@ -129,6 +128,7 @@ def __init__(
self._installed = installed
self._prefers_installed = prefers_installed
self._incompatible_ids = incompatible_ids
self._bool: Optional[bool] = None

def __getitem__(self, index: Any) -> Any:
# Implemented to satisfy the ABC check. This is not needed by the
Expand All @@ -152,8 +152,13 @@ def __len__(self) -> int:
# performance reasons).
raise NotImplementedError("don't do this")

@functools.lru_cache(maxsize=1)
def __bool__(self) -> bool:
if self._bool is not None:
return self._bool

if self._prefers_installed and self._installed:
self._bool = True
return True
return any(self)

self._bool = any(self)
return self._bool
3 changes: 2 additions & 1 deletion src/pip/_internal/resolution/resolvelib/provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -270,8 +270,9 @@ def _eligible_for_upgrade(identifier: str) -> bool:
is_satisfied_by=self.is_satisfied_by,
)

@staticmethod
@lru_cache(maxsize=None)
def is_satisfied_by(self, requirement: Requirement, candidate: Candidate) -> bool:
def is_satisfied_by(requirement: Requirement, candidate: Candidate) -> bool:
return requirement.is_satisfied_by(candidate)

def get_dependencies(self, candidate: Candidate) -> Iterable[Requirement]:
Expand Down
11 changes: 8 additions & 3 deletions tests/unit/test_finder.py
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,10 @@ def test_finder_priority_file_over_page(data: TestData) -> None:
find_links=[data.find_links],
index_urls=["http://pypi.org/simple/"],
)
all_versions = finder.find_all_candidates(req.name)
name = req.name
assert name == "gmpy"

all_versions = finder.find_all_candidates(name)
# 1 file InstallationCandidate followed by all https ones
assert all_versions[0].link.scheme == "file"
assert all(
Expand All @@ -335,9 +338,11 @@ def test_finder_priority_nonegg_over_eggfragments() -> None:
"""Test PackageFinder prefers non-egg links over "#egg=" links"""
req = install_req_from_line("bar==1.0")
links = ["http://foo/bar.py#egg=bar-1.0", "http://foo/bar-1.0.tar.gz"]
name = req.name
assert name == "bar"

finder = make_test_finder(links)
all_versions = finder.find_all_candidates(req.name)
all_versions = finder.find_all_candidates(name)
assert all_versions[0].link.url.endswith("tar.gz")
assert all_versions[1].link.url.endswith("#egg=bar-1.0")

Expand All @@ -349,7 +354,7 @@ def test_finder_priority_nonegg_over_eggfragments() -> None:
links.reverse()

finder = make_test_finder(links)
all_versions = finder.find_all_candidates(req.name)
all_versions = finder.find_all_candidates(name)
assert all_versions[0].link.url.endswith("tar.gz")
assert all_versions[1].link.url.endswith("#egg=bar-1.0")
found = finder.find_requirement(req, False)
Expand Down