-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Type hints and doc strings: conda.cli.main_info #13445
Conversation
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 is a great start! Only suggestion I have is to make the return types more specific.
Regarding reviewing the rendered docs, these are auto generated as part of PR checks/GH Actions (see the ReadTheDocs check below).
More specific dict type Co-authored-by: Ken Odegard <kodegard@anaconda.com>
pre-commit.ci autofix |
for more information, see https://pre-commit.ci
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.
I think this looks good. Waiting for @kenodegard to approve too before merging.
Thank you!
blocked by unrelated test failures, waiting on #13449 |
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.
Specifying the return type again in the docstring is a duplication of information and not needed because sphinx-autoapi supports autodoc's ability to automatically derive it from the object's typehints!
Thank you for updating the docstrings with blurbs of course and adding the typehints, but could you please remove the :returns:
statements?
I think we still want the |
Please correct me if I'm wrong but I believe the |
Co-authored-by: Ken Odegard <kodegard@anaconda.com>
Co-authored-by: Ken Odegard <kodegard@anaconda.com>
Co-authored-by: Ken Odegard <kodegard@anaconda.com>
Co-authored-by: Ken Odegard <kodegard@anaconda.com>
@samhaese you are correct, no need to remove |
pre-commit.ci autofix |
for more information, see https://pre-commit.ci
Description
This PR is part of an effort to add type hints and doc strings (#13437 ). This PR only modified conda.cli.main_info by adding type hints and rst-style doc strings, with one exception (changed the name of the argument for
dump_record
frompkg
toprec
).This is my first PR for this effort, so there were a few things that I'm not sure about. For example, how to check API documentation rendering as suggested in the issue. Advice and feedback appreciated :)
Miscellaneous notes
Going through the code for type hints and descriptions, I came across a few things that might deserve attention.
get_user_site
function be updated with platformdirs? Right now it returns possibly misleading information.conda info
usages have no --json supportChecklist - did you ...
news
directory (using the template) for the next release's release notes?