Skip to content

Fix past_key_values type hint in model output types #37953

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.

Sign up for GitHub

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

Jump to bottom
Merged
gante merged 5 commits into from
May 13, 2025
Merged

Fix past_key_values type hint in model output types #37953

gante merged 5 commits into from
May 13, 2025

Conversation

@ChengLyu
Copy link
Contributor

@ChengLyu ChengLyu commented May 5, 2025 •
edited
Loading

What does this PR do?

This PR fixes the past_key_values type hint in model output types to adopt the new Cache type.

Before submitting

  • [✅ ] This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • Did you read the contributor guideline,
    Pull Request section?
  • Was this discussed/approved via a Github issue or the forum? Please add a link
    to it if that's the case.
  • Did you make sure to update the documentation with your changes? Here are the
    documentation guidelines, and
    here are tips on formatting docstrings.
  • Did you write any new necessary tests?

Who can review?

@patrickvonplaten @sgugger @tanreinama

@github-actions github-actions bot marked this pull request as draft May 5, 2025 06:12
@github-actions
Copy link
Contributor

github-actions bot commented May 5, 2025

Hi 👋, thank you for opening this pull request! The pull request is converted to draft by default. The CI will be paused while the PR is in draft mode. When it is ready for review, please click the Ready for review button (at the bottom of the PR page). This will assign reviewers and trigger CI.

@ChengLyu ChengLyu marked this pull request as ready for review May 5, 2025 06:41
@ChengLyu ChengLyu force-pushed the fix-past-key-value-type-hint branch from 32dd36d to 536e039 Compare May 5, 2025 18:47
@Rocketknight1
Copy link
Member

Rocketknight1 commented May 6, 2025

LGTM but cc @gante to double-check this is right! Feel free to approve + merge without additional review if you're happy

@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented May 6, 2025

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.

gante
gante reviewed May 6, 2025
View reviewed changes
Copy link
Contributor

@gante gante left a comment

Choose a reason for hiding this comment

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

Looks mostly good to me, two nits :)

ArthurZucker
ArthurZucker approved these changes May 8, 2025
View reviewed changes
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, this does not "remove" old but rather properly updates the typing! thanks

@ChengLyu ChengLyu force-pushed the fix-past-key-value-type-hint branch from 186af6e to 5257fdf Compare May 8, 2025 17:51
@ChengLyu ChengLyu force-pushed the fix-past-key-value-type-hint branch from 6612363 to 8bb4f48 Compare May 8, 2025 19:34
gante
gante approved these changes May 13, 2025
View reviewed changes
Copy link
Contributor

@gante gante left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM, thank you for iterating 💛

@gante gante enabled auto-merge (squash) May 13, 2025 13:23
@gante gante merged commit 67b3d45 into huggingface:main May 13, 2025
20 checks passed
zucchini-nlp pushed a commit to zucchini-nlp/transformers that referenced this pull request May 14, 2025
@ChengLyu @zucchini-nlp
Fix past_key_values type hint in model output types (huggingface#37953
a59bedc 
)

* F: Fix type hint.

* F: Use Cache type.

* F: Sort import.

* U: Format.

* U: Address reviews.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ArthurZucker ArthurZucker ArthurZucker approved these changes

@Rocketknight1 Rocketknight1 Awaiting requested review from Rocketknight1

+2 more reviewers

@gante gante gante approved these changes

@tanreinama tanreinama tanreinama 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.

6 participants

@ChengLyu @Rocketknight1 @HuggingFaceDocBuilderDev @gante @ArthurZucker @tanreinama