Skip to content

[Tests] Update a deprecated parameter in test files and fix several typos#7277

Merged
stevhliu merged 20 commits into
huggingface:mainfrom
tolgacangoz:fix-typos
Mar 14, 2024
Merged

[Tests] Update a deprecated parameter in test files and fix several typos#7277
stevhliu merged 20 commits into
huggingface:mainfrom
tolgacangoz:fix-typos

Conversation

@tolgacangoz
Copy link
Copy Markdown
Contributor

@tolgacangoz tolgacangoz commented Mar 11, 2024 •
edited
Loading

This pull request updates the deprecated output_type="numpy" to output_type="np" in the test files, and fixes several typos.
@stevhliu

@tolgacangoz tolgacangoz changed the title [Tests] Update deprecated output_type in test files [Tests] Update deprecated output_type="numpy" in test files Mar 11, 2024
@tolgacangoz tolgacangoz changed the title [Tests] Update deprecated output_type="numpy" in test files [Tests] Update deprecated output_type="numpy" in test files and fix typos Mar 11, 2024
@tolgacangoz
Copy link
Copy Markdown
Contributor Author

tolgacangoz commented Mar 12, 2024 •
edited
Loading

Sorry for the unrelated commit history. I reversed them.

@tolgacangoz tolgacangoz changed the title [Tests] Update deprecated output_type="numpy" in test files and fix typos [Tests] Update deprecated output_type="numpy" in test files and fix several typos Mar 12, 2024
stevhliu
stevhliu reviewed Mar 12, 2024
View reviewed changes
Copy link
Copy Markdown
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.

Hi, the change from output_type="numpy" to "np" looks good but let's not change w to timesteps because it refers to guidance scale which is appropriate here.

@HuggingFaceDocBuilderDev
Copy link
Copy Markdown

HuggingFaceDocBuilderDev commented Mar 12, 2024

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.

@tolgacangoz
Copy link
Copy Markdown
Contributor Author

tolgacangoz commented Mar 12, 2024 •
edited
Loading

Yeah, that is not necessarily about timesteps. I am updating the docstring then?

@tolgacangoz tolgacangoz requested a review from stevhliu March 12, 2024 18:42
@stevhliu
Copy link
Copy Markdown
Member

stevhliu commented Mar 12, 2024

Yeah, let's just keep the changes that are related to numpy --> np

@tolgacangoz
Copy link
Copy Markdown
Contributor Author

tolgacangoz commented Mar 12, 2024 •
edited
Loading

I couldn't understand. Should I change timesteps to w in the get_guidance_scale_embedding()'s docstring? Plus, in the code, of course.

@stevhliu
Copy link
Copy Markdown
Member

stevhliu commented Mar 13, 2024

Sorry I wasn't clear before! Let's keep w in the code and change the docstring from timesteps to w.

  1. Keep w in the code and change the docstring from timesteps to w.
  2. Update the parameter description for it. Currently, it says "generate embedding vectors at these timesteps" but that's not the clearest or most accurate. Instead, let's say something like: "Generate embedding vectors with a specified guidance scale to subsequently enrich timestep embeddings."

@tolgacangoz
Copy link
Copy Markdown
Contributor Author

tolgacangoz commented Mar 13, 2024 •
edited
Loading

There are lots of other deprecation warnings in tests. Can/Should I open another PR(s) to update them?

@tolgacangoz tolgacangoz changed the title [Tests] Update deprecated output_type="numpy" in test files and fix several typos [Tests] Update several deprecated parameters in test files and fix several typos Mar 13, 2024
@stevhliu
Copy link
Copy Markdown
Member

stevhliu commented Mar 13, 2024 •
edited
Loading

Yeah you can open a separate PR for the deprecation warnings. It'd be better to keep the focus of this PR on just numpy to np and updating the docstring from timestep to w with a clearer parameter description.

@tolgacangoz tolgacangoz changed the title [Tests] Update several deprecated parameters in test files and fix several typos [Tests] Update a deprecated parameter in test files and fix several typos Mar 14, 2024
stevhliu
stevhliu approved these changes Mar 14, 2024
View reviewed changes
Copy link
Copy Markdown
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.

One last change and then we should be ready to merge :)

Comment thread src/diffusers/pipelines/controlnet/pipeline_controlnet.py Outdated
@tolgacangoz tolgacangoz requested a review from stevhliu March 14, 2024 17:26
stevhliu
stevhliu approved these changes Mar 14, 2024
View reviewed changes
Copy link
Copy Markdown
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!

@stevhliu stevhliu merged commit 5d848ec into huggingface:main Mar 14, 2024
@tolgacangoz
Copy link
Copy Markdown
Contributor Author

tolgacangoz commented Mar 14, 2024

Thanks for reviewing and merging!

@tolgacangoz tolgacangoz deleted the fix-typos branch March 14, 2024 19:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@stevhliu stevhliu stevhliu approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@tolgacangoz @HuggingFaceDocBuilderDev @stevhliu