Skip to content

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