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

Add docstrings #219

Closed
mjspeck opened this issue Dec 22, 2023 · 10 comments
Closed

Add docstrings #219

mjspeck opened this issue Dec 22, 2023 · 10 comments

Comments

@mjspeck
Copy link

mjspeck commented Dec 22, 2023

Obviously this would be pretty big for a single issue but that fact that most methods don't seem to have docstrings is troubling. The tutorials you have are very helpful but they're not a full replacement for actual docstrings, type annotations, etc. which make it much easier to build using llmware. Happy to contribute as well, but that would require knowing what docstring format y'all prefer and other style guide standards.

@rahpandey1
Copy link

I can also contribute

@doberst
Copy link
Contributor

doberst commented Jan 21, 2024

@mjspeck and @rahpandey1 - appreciate this feedback. We have taken initial steps on docstrings - very simple and lightweight through the main code. Realize that there is a lot more to do here - would welcome ideas and help to improve on this starting point. On type annotations for Python, we have debated it internally quite a bit - and right now, would probably side with prioritizing docstrings as well as core documentation.

@MacOS
Copy link
Contributor

MacOS commented Jan 23, 2024

I'm also happy to contribute on this!

@MacOS
Copy link
Contributor

MacOS commented Feb 8, 2024

I would suggest to add docstrings systematically in the following order, where one point is one issues and hence one pull request.

  1. Add docstrings to all modules,
  2. Add docstrings to all classes and functions,
  3. Add docstrings to all methods and class methods.

@MacOS
Copy link
Contributor

MacOS commented Feb 14, 2024

I submitted the pull request #406 that adds docstrings to all modules, and the package.

After this PR has been merged, we can move forward by adding docstrings to the classes.

@MacOS
Copy link
Contributor

MacOS commented Feb 26, 2024

I submitted the pull request #449 to add class docstrings to four modules. I decided to split up adding class docstrings, since one pull request that adds class docstrings to all classes would be too large.

@MacOS
Copy link
Contributor

MacOS commented Feb 26, 2024

To better organise the documentation effort, here is a list of what needs to be done, including what has already been done and what still needs to be done.

@mjspeck
Copy link
Author

mjspeck commented Feb 26, 2024

Sorry I haven't been active on this. I'll take a look in the next week at what I can contribute.

@MacOS
Copy link
Contributor

MacOS commented Feb 28, 2024

@mjspeck No worries. Please post first on what you are working before you start. I would recommend that you pick on module and add class docstrings to all classes there, and then submit this as a PR. Otherwise, the PR gets too big.

@doberst
Copy link
Contributor

doberst commented May 5, 2024

Docstrings substantially complete throughout the code base. If specific request, we can open up as a new issue.

@doberst doberst closed this as completed May 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants