Skip to content

Fix two tiny typos in pipelines/base.py::Pipeline::_sanitize_parameters()'s docstring#29102

Merged
amyeroberts merged 2 commits into
huggingface:mainfrom
sadra-barikbin:patch-4
Feb 19, 2024
Merged

Fix two tiny typos in pipelines/base.py::Pipeline::_sanitize_parameters()'s docstring#29102
amyeroberts merged 2 commits into
huggingface:mainfrom
sadra-barikbin:patch-4

Conversation

@sadra-barikbin

@sadra-barikbin sadra-barikbin commented Feb 19, 2024

Copy link
Copy Markdown
Contributor

Hi there!
To fix two tiny typos in pipelines/base.py::Pipeline::_sanitize_parameters()'s docstring.

@Narsil

amyeroberts
amyeroberts approved these changes Feb 19, 2024
View reviewed changes

@amyeroberts amyeroberts left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for fixing!

@sadra-barikbin

sadra-barikbin commented Feb 19, 2024

Copy link
Copy Markdown
Contributor Author

One more thing. We have False for default value of clean_up_tokenization_spaces argument in TextGenerationPipeline's docstring:

transformers/src/transformers/pipelines/text_generation.py

Lines 207 to 208 in 593230f

clean_up_tokenization_spaces (`bool`, *optional*, defaults to `False`):
Whether or not to clean up the potential extra spaces in the text output.

but its default value in TextGenerationPipeline::postprocess is True:

transformers/src/transformers/pipelines/text_generation.py

Line 336 in 593230f

def postprocess(self, model_outputs, return_type=ReturnType.FULL_TEXT, clean_up_tokenization_spaces=True):

@HuggingFaceDocBuilderDev

HuggingFaceDocBuilderDev commented Feb 19, 2024

Copy link
Copy Markdown

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.

@amyeroberts

amyeroberts commented Feb 19, 2024

Copy link
Copy Markdown
Contributor

@sadra-barikbin Good spot! Could you add a fix for the docstring in this PR too?

@sadra-barikbin

sadra-barikbin commented Feb 19, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

The pipelines in text2text_generation.py class have clean_up_tokenization_spaces=False as default. This has been the desired behavior?

@amyeroberts

amyeroberts commented Feb 19, 2024

Copy link
Copy Markdown
Contributor

@sadra-barikbin Yes, I believe so. Changing the default behaviour would be a breaking change, so we should keep as-is

@amyeroberts
amyeroberts merged commit a4851d9 into huggingface:main Feb 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@amyeroberts amyeroberts amyeroberts approved these changes
Reviewers whose approvals may not affect merge requirements

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

@sadra-barikbin @HuggingFaceDocBuilderDev @amyeroberts