solve module cleanup #5502
solve module cleanup #5502
Conversation
conda/core/solve.py
Outdated
| state. | ||
|
|
||
| """ | ||
|
|
There was a problem hiding this comment.
What is the above string doing here ? Is it meant as a reminder (to the implementer) what the code should do ? Given that it's a clone of condas CLI help output (and thus is encoded elsewhere), there is a non-zero risk that it will get out-of-sync, so I would prefer not to duplicate it here.
conda/core/solve.py
Outdated
|
|
||
|
|
||
| class Solver(object): | ||
|
|
There was a problem hiding this comment.
Assuming that this is going to be part of the public API, the class itself should use a docstring, too.
conda/core/solve.py
Outdated
| prune (bool): | ||
| If ``True``, the solution will not contain packages that were | ||
| previously brought into the environment as dependencies but are no longer | ||
| required as dependencies and are not user-requested. |
There was a problem hiding this comment.
What is the meaning of the default value 'NULL' ? (The same question applies to the other arguments, too.)
conda/core/solve.py
Outdated
| link_dists = tuple(Dist(d) for d in link_dists) | ||
| stp = PrefixSetup(self.index, self.prefix, unlink_dists, link_dists, 'INSTALL', | ||
| self.specs_to_add) | ||
| return UnlinkLinkTransaction(stp) |
There was a problem hiding this comment.
This separation into the three distinct solve_... methods looks great ! Why not go a step further and remove the second (solve_for_diff) and third (solve_for_transaction) from the Solver class and make them simple functions whose input is the output of Solver.solve_final_state ? It seems to me that would simplify the API as then the Solver wouldn't have to know anything about files, links, and transactions. It would simply be a computation device that transforms certain models (such as MatchSpec and PackageRecord) into other models (PackageRef).
There was a problem hiding this comment.
Since this PR overlaps with the RepoData API work (#5267) I think it's best to complete the latter first (given that we already went through a review), then rebase and adjust this one to use the new features.
For example, it isn't clear why the Solver constructor needs to take channel arguments, given that the RepoData already carries that information in its global state.
Talk in person, and Stefan is now ok moving forward.
|
Incorporated into #5546. Closing. |
|
Hi there, thank you for your contribution to Conda! This pull request has been automatically locked since it has not had recent activity after it was closed. Please open a new issue or pull request if needed. |
No description provided.