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.

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

Always prefer causes when backtracking #132

Closed
Closed
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
11 changes: 1 addition & 10 deletions src/resolvelib/providers.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,7 @@ def identify(self, requirement_or_candidate):
"""
raise NotImplementedError

def get_preference(
self,
identifier,
resolutions,
candidates,
information,
backtrack_causes,
):
def get_preference(self, identifier, resolutions, candidates, information):
Copy link
Member

@uranusjr uranusjr Aug 18, 2023

Choose a reason for hiding this comment

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

We’re at version 1 now so it’d be better if we could implement this in a backward-compatible way. Maybe we can still pass the backtrack causes anyway? We could potentially mark it as deprecated and pending removal in 2.0.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure, it was just to simplify the interface and make sure downstream libraries aren't duplicating this logic, but I don't have any strong opinions on whether to keep providing it or not.

"""Produce a sort key for given requirement based on preference.

The preference is defined as "I think this requirement should be
Expand All @@ -32,8 +25,6 @@ def get_preference(
Each value is an iterator of candidates.
:param information: Mapping of requirement information of each package.
Each value is an iterator of *requirement information*.
:param backtrack_causes: Sequence of requirement information that were
the requirements that caused the resolver to most recently backtrack.

A *requirement information* instance is a named tuple with two members:

Expand Down
1 change: 0 additions & 1 deletion src/resolvelib/providers.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ class AbstractProvider(Generic[RT, CT, KT]):
resolutions: Mapping[KT, CT],
candidates: Mapping[KT, Iterator[CT]],
information: Mapping[KT, Iterator[RequirementInformation[RT, CT]]],
backtrack_causes: Sequence[RequirementInformation[RT, CT]],
) -> Preference: ...
def find_matches(
self,
Expand Down
9 changes: 9 additions & 0 deletions src/resolvelib/reporters.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,12 @@ def rejecting_candidate(self, criterion, candidate):

def pinning(self, candidate):
"""Called when adding a candidate to the potential solution."""

def backtracking_on(self, names, unsatisfied_names):
"""Called when resolver identifies specific projects causing backtracking.

:param causes: The names of projects causing the backtracking.
:param causes: The names of projects and their parents that are
currently unsatisfied by the resolver.

"""
3 changes: 3 additions & 0 deletions src/resolvelib/reporters.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,6 @@ class BaseReporter:
def rejecting_candidate(self, criterion: Any, candidate: Any) -> Any: ...
def resolving_conflicts(self, causes: Any) -> Any: ...
def pinning(self, candidate: Any) -> Any: ...
def backtracking_on(
self, names: set[str], unsatisfied_names: set[str]
) -> Any: ...
24 changes: 21 additions & 3 deletions src/resolvelib/resolvers.py
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,6 @@ def _get_preference(self, name):
self.state.criteria,
operator.attrgetter("information"),
),
backtrack_causes=self.state.backtrack_causes,
)

def _is_current_pin_satisfying(self, name, criterion):
Expand Down Expand Up @@ -418,10 +417,29 @@ def resolve(self, requirements, max_rounds):
return self.state

# keep track of satisfied names to calculate diff after pinning
satisfied_names = set(self.state.criteria.keys()) - set(
unsatisfied_names
unsatisfied_names_set = set(unsatisfied_names)
satisfied_names = (
set(self.state.criteria.keys()) - unsatisfied_names_set
)

# If backtrack causes prefer them and their parents first
if self.state.backtrack_causes:
backtrack_cause_names = set()
for backtrack_cause in self.state.backtrack_causes:
backtrack_cause_names.add(backtrack_cause.requirement.name)
if backtrack_cause.parent:
backtrack_cause_names.add(backtrack_cause.parent.name)

unsatisfied_causes_names = unsatisfied_names_set & (
backtrack_cause_names
)
if unsatisfied_causes_names:
unsatisfied_names = list(unsatisfied_causes_names)

self._r.backtracking_on(
backtrack_cause_names, unsatisfied_causes_names
)

# Choose the most preferred unpinned criterion to try.
name = min(unsatisfied_names, key=self._get_preference)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I could put a check here to see if the length of unsatisfied names is 1 and skip calling self._get_preference altogether under this scenario?

failure_causes = self._attempt_to_pin_criterion(name)
Expand Down
9 changes: 1 addition & 8 deletions tests/functional/cocoapods/test_resolvers_cocoapods.py
Original file line number Diff line number Diff line change
Expand Up @@ -174,14 +174,7 @@ def __init__(self, filename):
def identify(self, requirement_or_candidate):
return requirement_or_candidate.name

def get_preference(
self,
identifier,
resolutions,
candidates,
information,
backtrack_causes,
):
def get_preference(self, identifier, resolutions, candidates, information):
return sum(1 for _ in candidates[identifier])

def _iter_matches(self, name, requirements, incompatibilities):
Expand Down
9 changes: 1 addition & 8 deletions tests/functional/python/test_resolvers_python.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,14 +71,7 @@ def identify(self, requirement_or_candidate):
return "{}[{}]".format(name, extras_str)
return name

def get_preference(
self,
identifier,
resolutions,
candidates,
information,
backtrack_causes,
):
def get_preference(self, identifier, resolutions, candidates, information):
transitive = all(p is not None for _, p in information[identifier])
return (transitive, identifier)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,14 +78,7 @@ def __init__(self, filename):
def identify(self, requirement_or_candidate):
return requirement_or_candidate.container["identifier"]

def get_preference(
self,
identifier,
resolutions,
candidates,
information,
backtrack_causes,
):
def get_preference(self, identifier, resolutions, candidates, information):
return sum(1 for _ in candidates[identifier])

def _iter_matches(self, identifier, requirements, incompatibilities):
Expand Down