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

Fix some documentation in ./src/diffusers/models/adapter.py #9591

Merged
merged 34 commits into from
Oct 15, 2024

Conversation

ahnjj
Copy link
Contributor

@ahnjj ahnjj commented Oct 5, 2024

What does this PR do?

Fixes #9567

Fixes # (issue)

  • Unified two expressions with the same meaning into one. (arguments / parameters -> args)
  • Concise Summary and argument descriptions
  • Corrected Grammar

Before submitting

Who can review?

This PR tries to attempt a solution at one of the submodules listed in #9567 so I think @a-r-r-o-w is the best to review it. Alongside the same, @charchit7 @yijun-lee and @SubhasmitaSw were also working on the same, so just a ping for the update on the same.

@ahnjj ahnjj marked this pull request as ready for review October 5, 2024 07:54
src/diffusers/models/adapter.py Outdated Show resolved Hide resolved
src/diffusers/models/adapter.py Outdated Show resolved Hide resolved
src/diffusers/models/adapter.py Outdated Show resolved Hide resolved
src/diffusers/models/adapter.py Outdated Show resolved Hide resolved
src/diffusers/models/adapter.py Outdated Show resolved Hide resolved
src/diffusers/models/adapter.py Outdated Show resolved Hide resolved
src/diffusers/models/adapter.py Outdated Show resolved Hide resolved
src/diffusers/models/adapter.py Outdated Show resolved Hide resolved
src/diffusers/models/adapter.py Outdated Show resolved Hide resolved
src/diffusers/models/adapter.py Outdated Show resolved Hide resolved
@yiyixuxu yiyixuxu requested a review from stevhliu October 9, 2024 01:57
Copy link
Member

@stevhliu stevhliu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for improving, running make style should fix the failing CI tests!

src/diffusers/models/adapter.py Outdated Show resolved Hide resolved
src/diffusers/models/adapter.py Outdated Show resolved Hide resolved
src/diffusers/models/adapter.py Outdated Show resolved Hide resolved
src/diffusers/models/adapter.py Outdated Show resolved Hide resolved
src/diffusers/models/adapter.py Outdated Show resolved Hide resolved
src/diffusers/models/adapter.py Outdated Show resolved Hide resolved
src/diffusers/models/adapter.py Outdated Show resolved Hide resolved
ahnjj and others added 8 commits October 12, 2024 15:01
Co-authored-by: Steven Liu <59462357+stevhliu@users.noreply.github.com>
Co-authored-by: Steven Liu <59462357+stevhliu@users.noreply.github.com>
Co-authored-by: Steven Liu <59462357+stevhliu@users.noreply.github.com>
Co-authored-by: Steven Liu <59462357+stevhliu@users.noreply.github.com>
Co-authored-by: Steven Liu <59462357+stevhliu@users.noreply.github.com>
Co-authored-by: Steven Liu <59462357+stevhliu@users.noreply.github.com>
Co-authored-by: Steven Liu <59462357+stevhliu@users.noreply.github.com>
@a-r-r-o-w
Copy link
Member

Hey, seeing some quality tests failing. Could you run make style and push the changes?

@HuggingFaceDocBuilderDev

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

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

It seems like you're on a different version of ruff than what we use on our CI, causing unrelated changes. Could you make sure your ruff version is 0.1.5, run make style again and push the changes? Sorry about the inconvenience!

@ahnjj
Copy link
Contributor Author

ahnjj commented Oct 15, 2024

It seems like you're on a different version of ruff than what we use on our CI, causing unrelated changes. Could you make sure your ruff version is 0.1.5, run make style again and push the changes? Sorry about the inconvenience!

Thanks for your help!
I installed 0.1.5 version ruff, run make style and it shows an error with 'doc-builder'.
Do you happen to know regarding this? Thank you so much!

make: doc-builder: Command not found
make: *** [Makefile:59: style] Error 127

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

Could you try pip install hf-doc-builder? We use this for building docs. Although, I don't think you need to build docs locally (as it's automatically built by our CI workflows), so just pushing the relevant quality fixes with the correct ruff version is required (because, right now, it's over 3000+ changes now with a different version). Let me know if you're facing any problems, and I'd be happy to push from my end :)

@ahnjj
Copy link
Contributor Author

ahnjj commented Oct 15, 2024

so just pushing the relevant quality fixes with the correct ruff version is required (because, right now, it's over 3000+ changes now with a different version). Let me know if you're facing any problems, and I'd be happy to push from my end :)

Thank you so much with your help!
I made adjustment ( with 0.1.5 version ruff)
Please let me know if anything left for merge! : >

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

@stevhliu Good to merge? All tests are passing now

@stevhliu stevhliu merged commit d40da7b into huggingface:main Oct 15, 2024
15 checks passed
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

Successfully merging this pull request may close these issues.

[community] Improving docstrings and type hints
4 participants