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
resolve #5610 add PrefixData, SubdirData, and PackageCacheData to conda/api.py #6922
Conversation
conda/api.py
Outdated
DepsModifier = _DepsModifier | ||
|
||
|
||
class Solver(object): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need a docstring here, that also explains why this Solver
wrapper is needed, or intended for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any recommendation on how to do this without duplicating everything that's already in the underlying Solver
class? Plenty of nice docstrings here: https://github.com/conda/conda/blob/master/conda/core/solve.py#L52
conda/api.py
Outdated
|
||
|
||
class SubdirData(object): | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need a docstring here, that also explains why this Solver
wrapper is needed, or intended for?
conda/api.py
Outdated
|
||
|
||
class PackageCacheData(object): | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good in general, but being in the api and exposed to the "public" we will need to document the class wrappers and all the methods, with examples of usage.
If we want this incorporated into conda 4.5, we might have to move on it without docstrings. Conda 4.5.0 needs to be in canary tomorrow. If we don't mind bumping it to conda 4.6, then we can add docstrings. |
conda/api.py
Outdated
class Solver(object): | ||
|
||
def __init__(self, prefix, channels, subdirs=(), specs_to_add=(), specs_to_remove=()): | ||
self._internal = _Solver(prefix, channels, subdirs, specs_to_add, specs_to_remove) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will only work for interactive scripting but we could do at least
self.solve_final_state.__doc__ = self._internal.solve_final_state.__doc__
Ok, no solid examples yet, but pretty solid documentation. Is that a reasonable compromise to get this into 4.5.0? |
I think we should add "beta" tags to every docstring in |
@msarahan has done a quick review. We discussed needing to add a way to add individual packages to repodata. That's beyond the scope of this PR, as it'll take some work to the underlying |
merging based on approval by @goanpeca |
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. |
resolve #5610