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

[community] Improving docstrings and type hints #9567

Open
a-r-r-o-w opened this issue Oct 2, 2024 · 13 comments · May be fixed by #9579, #9582, #9583, #9590 or #9591
Open

[community] Improving docstrings and type hints #9567

a-r-r-o-w opened this issue Oct 2, 2024 · 13 comments · May be fixed by #9579, #9582, #9583, #9590 or #9591
Labels
contributions-welcome documentation Improvements or additions to documentation good first issue Good for newcomers

Comments

@a-r-r-o-w
Copy link
Member

There are many instances in the codebase where our docstring/typing convention is not followed. We'd like to work on improving this with your help!

Our convention looks like:

def function_name(parameter_1: Union[str, List[str]], parameter_2: Optional[int] = None, parameter_3: float = 42.0) -> Civilization:
    r"""
    Function that creates a simulation.

    Args:
        parameter_1 (`str` or `List[str]`):
            Description of game level.
        parameter_2 (`int`, *optional*):
            Kardashev scale of civilization.
        parameter_3 (`float`, defaults to `42.0`):
            Difficulty scale.

    Returns:
        [`~simulations.objects.Civilization`]
            A civilization simulation with provided initialization parameters.
    """

Some examples that don't follow the docstring convention are:

  • this: missing explanations
  • this: does not contain mixin-related documentation whereas as this does
  • this: function explanation after "Args", but should be before
  • this: same reason as above
  • this: incorrect indentation

There are also many places where docstrings are completely missing or inadequately explained. If you feel something needs an improvement, you can open a PR with your suggestions too! Additionally, type hints are not appropriate/correctly used at many occurrences and mismatch the accompanying docstrings - these could use an improvement too!

Please limit your PRs to changes to a single file in each PR. Changes must be only related to docstrings/type hints. Feel free to ping either @yiyixuxu, @stevhliu or me for reviews.

@a-r-r-o-w a-r-r-o-w added documentation Improvements or additions to documentation good first issue Good for newcomers contributions-welcome labels Oct 2, 2024
@SubhasmitaSw
Copy link

Hi @a-r-r-o-w I'd like to take this up, please let me know if there are any other prerequisites I should be aware of before submitting a PR against this issue 🙂

@a-r-r-o-w
Copy link
Member Author

Not prerequisites I can think of off the top of my head. Just that the PRs should be limited in scope as mentioned. You can maybe look at the Diffusers contribution guide (and philosophy, if you're interested)

@charchit7
Copy link
Contributor

I'll take up some of these.

@yijun-lee
Copy link

I’m also interested in this work. Could you let me know about the current progress? @SubhasmitaSw @charchit7

@charchit7
Copy link
Contributor

charchit7 commented Oct 4, 2024

Hey @yijun-lee, I've been caught up with some work, unfortunately, but I’ll work on this over the weekend or later today. If you want to get started on any of these tasks, feel free to go ahead and let us know, so we can pick up whatever is left.

@yijun-lee
Copy link

Oh, actually, I think I’ll be starting this weekend as well. If we proceed separately, it would be good to inform each other through comments or other means. Have a good day :) @charchit7

@charchit7
Copy link
Contributor

Sure, @yijun-lee, that works!
you too :)

@DTG2005
Copy link

DTG2005 commented Oct 4, 2024

Hello there guys, I'd also like to contribute in this issue. I'm sorry I didn't really drop in a message here yet but I hope this PR helps push things forward! A g'day to all.

@a-r-r-o-w
Copy link
Member Author

Feel free to take up as many files as you want (one file per PR however)! The ones mentioned in the issue description are just a few examples, but there are probably hundreds of files that could use improvements. Please keep them coming, thanks

@jeongiin
Copy link

jeongiin commented Oct 5, 2024

Hello! I'm also following this issue with interest. I’ve submitted my first PR, so please let me know if there are any mistakes! Have a great day!

@ahnjj ahnjj linked a pull request Oct 5, 2024 that will close this issue
6 tasks
@ahnjj
Copy link

ahnjj commented Oct 5, 2024

Hello! Thanks for holding interesting issue! I'm fully circled for this new work ! 🙆🏻‍♀️ 🙆🏻‍♀️
I also have opened my PR, please let me know if I missed something !

  • Q. which should I prioritize modern python docstring conventions or unity of that file (e.g. expression) ?

@Ashutoshjangam
Copy link

Ashutoshjangam commented Oct 5, 2024

@a-r-r-o-w hi, i wank to work on it

@Jwaminju
Copy link

Jwaminju commented Oct 5, 2024

Hello colleagues,
I'm also interested into this issues. I made a first PR related to this issue.
Since there are lots of docstrings to update, I'm also interested into list up missing docstring files if I have time :)
Thank you in advance for your time and guidance!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment