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

Update Docstrings #1216

Open
5 of 8 tasks
Tracked by #1198
guarin opened this issue May 10, 2023 · 10 comments
Open
5 of 8 tasks
Tracked by #1198

Update Docstrings #1216

guarin opened this issue May 10, 2023 · 10 comments

Comments

@guarin
Copy link
Contributor

guarin commented May 10, 2023

Description

Go through all PIP modules and make sure that the docstrings are consistently formatted and complete.

  • We only care about public functions/classes/methods for now (things that are in the Python API Docs).
  • We don't care about outdated/deprecated modules such as active_learning and the high level models.

This is an example of a well documented file: https://github.com/lightly-ai/lightly/blob/master/lightly/models/modules/memory_bank.py

When updating docstrings or comments keep the following in mind:

  • Follow the google python styleguide: https://google.github.io/styleguide/pyguide.html#38-comments-and-docstrings
  • Do not document default values if they are part of the function definition. E.g. if the function is defined as: def fun(value: int = 5) there is no need to document that 5 is the default for value in the docstring. It is already visible in the function definition. However, if the function is defined as def fun(value: int | None = None) and in the code we do: if value is None: value = 5 we should document that value is set to 5 by default as this is not evident from the function definition.
  • Make full sentences when writing comments
  • Do not add comments if it is evident from the code what is happening

How to work on this issue

  1. Take any file from one of the modules listed below
  2. Make sure that all public functions/classes/methods are well documented. You can also split the work into multiple PRs for files with many functions or classes with many methods.
  3. Add documentation if necessary
  4. Create a PR named "Add documentation for " and push your changes

Tasks

@guarin guarin mentioned this issue May 10, 2023
18 tasks
@guarin guarin added this to Cleanup Aug 25, 2023
@guarin guarin changed the title Cleanup docstrings Cleanup Docstrings Aug 16, 2024
@guarin guarin changed the title Cleanup Docstrings Update Docstrings Sep 11, 2024
@ayush22iitbhu
Copy link
Contributor

@guarin Is this issue resolved? If not then please assign it to me. I would love to work on this.

@guarin
Copy link
Contributor Author

guarin commented Oct 3, 2024

Hi and thanks for working on this! Could you quickly indicate on which files you would work?

@ayush22iitbhu
Copy link
Contributor

lightly/models/utils.py

@guarin
Copy link
Contributor Author

guarin commented Oct 3, 2024

Awesome, created the issue for you: #1663 If you comment on it I can assign it to you.

@Prathamesh010
Copy link
Contributor

Please create a issue for Cleanup docstrings in lightly/embedding subpackage and assign it to me. @guarin

@guarin
Copy link
Contributor Author

guarin commented Oct 7, 2024

Please create a issue for Cleanup docstrings in lightly/embedding subpackage and assign it to me. @guarin

Created the issue, if you comment on it I'll be able to assign it to you

@ayush22iitbhu
Copy link
Contributor

@guarin I want to work on lightly/loss subpackage, lightly/data subpackage, and lightly/utils subpackage. Please assign it to me.

@guarin
Copy link
Contributor Author

guarin commented Oct 7, 2024

@ayush22iitbhu I created the issue, I can assign it to you if you comment on it. #1673

I'll keep the others unassigned for now.

@fadkeabhi
Copy link
Contributor

@guarin I want to work on lightly/models/modules subpackage. Can you assign it to me.

@guarin
Copy link
Contributor Author

guarin commented Oct 9, 2024

Awesome, can you comment on the issue #1687 then I can assign it to you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

No branches or pull requests

4 participants