diffusers icon indicating copy to clipboard operation
diffusers copied to clipboard

[community] Improving docstrings and type hints

Open a-r-r-o-w opened this issue 1 year ago • 15 comments

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 avatar Oct 02 '24 03:10 a-r-r-o-w

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 🙂

whtssub avatar Oct 02 '24 06:10 whtssub

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)

a-r-r-o-w avatar Oct 02 '24 07:10 a-r-r-o-w

I'll take up some of these.

charchit7 avatar Oct 02 '24 12:10 charchit7

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

yijun-lee avatar Oct 04 '24 00:10 yijun-lee

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.

charchit7 avatar Oct 04 '24 03:10 charchit7

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

yijun-lee avatar Oct 04 '24 03:10 yijun-lee

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

charchit7 avatar Oct 04 '24 03:10 charchit7

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.

DTG2005 avatar Oct 04 '24 11:10 DTG2005

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

a-r-r-o-w avatar Oct 04 '24 12:10 a-r-r-o-w

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!

jeongiin avatar Oct 05 '24 07:10 jeongiin

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) ?

ahnjj avatar Oct 05 '24 08:10 ahnjj

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

Ashutoshjangam avatar Oct 05 '24 11:10 Ashutoshjangam

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!

Jwaminju avatar Oct 05 '24 18:10 Jwaminju

Hello everyone! 😊 I'm also interested in this issue and have made some updates to the docstrings in src/diffusers/training_utils.py. I would love to get your feedback on my changes!

I’m excited to contribute and be part of this discussion. Thank you in advance for your time and guidance 🤗

mreraser avatar Oct 08 '24 12:10 mreraser

Hi I would love to be of help here. I have made some additions to the docstrings in src/diffusers/training_utils.py. Would love to get your feedback on the PR :)

RogerSinghChugh avatar Oct 20 '24 13:10 RogerSinghChugh

Hi 👋

I'm working on improving the docstrings and type hints for all scheduler files. I've already opened two PRs:

  • #12622 - scheduling_ddim.py
  • #12623 - scheduling_amused.py

I plan to continue with the other schedulers in the src/diffusers/schedulers/ directory. I'll submit one PR per file as recommended.

Let me know if there are any specific schedulers you'd like me to prioritize or if you have any feedback on my approach. Thanks!

delmalih avatar Nov 10 '25 16:11 delmalih