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

Update logits_process.py docstrings to clarify penalty and reward cases (attempt #2) #26784

Merged
merged 2 commits into from
Oct 17, 2023

Conversation

larekrow
Copy link
Contributor

This PR fixes point 3 of #25970 by clarifying the penalty and reward cases for RepetitionPenaltyLogitsProcessor and EncoderRepetitionPenaltyLogitsProcessor within the docstrings.

PR #26129 was the original copy, but I have accidentally deleted my repo that submitted the PR, so I cannot reopen that PR 🙇

@gante, for your review please.

Copy link
Collaborator

@ArthurZucker ArthurZucker left a comment

Choose a reason for hiding this comment

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

LGTM thanks 😉

@ArthurZucker
Copy link
Collaborator

Could you run make fixup to pass the failing test

@larekrow
Copy link
Contributor Author

Hey @ArthurZucker, I tried to run make fixup but encountered an error in repo-consistency.

Traceback (most recent call last):
  File "/home/users/user/temp/transformers/utils/update_metadata.py", line 337, in <module>
    check_pipeline_tags()
  File "/home/users/user/temp/transformers/utils/update_metadata.py", line 316, in check_pipeline_tags
    model = model[0]
IndexError: tuple index out of range
make: *** [Makefile:44: repo-consistency] Error 1

Anyway I thought I should not be encountering this in the first place since I only modified docstrings in a single file. So, I ran black src/transformers/generation/logits_process.py which gave:

All done! ✨ 🍰 ✨
1 file left unchanged.

Even ruff src/transformers/generation/logits_process.py --fix does nothing. Not sure what I should do to pass the failing test? I'm using black==23.9.1 and ruff==0.0.292 if that helps.

@ArthurZucker
Copy link
Collaborator

Hey! we have "ruff>=0.0.241,<=0.0.259" so the version you are using is probably too far! Try downgrading or doing something likepip install -e ".[quality]"and then runmake style`

@larekrow
Copy link
Contributor Author

Thanks for the guidance @ArthurZucker! The test has passed 😄

@ArthurZucker
Copy link
Collaborator

Cool thanks for the contribution

@HuggingFaceDocBuilderDev

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint.

@ArthurZucker ArthurZucker merged commit 0b8604d into huggingface:main Oct 17, 2023
8 checks passed
EduardoPach pushed a commit to EduardoPach/transformers that referenced this pull request Nov 19, 2023
…es (attempt #2) (huggingface#26784)

* Update logits_process.py docstrings + match arg fields to __init__'s

* Ran `make style`
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.

3 participants