-
Notifications
You must be signed in to change notification settings - Fork 264
Modernize type hints - dataclass arguments and recipe metadata #1950
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
Modernize type hints - dataclass arguments and recipe metadata #1950
Conversation
👋 Hi! Thank you for contributing to llm-compressor. Please add the ready label when the PR is ready for review. Note: This is required to complete the testing suite, please only add the label once the PR is code complete and local testing has been performed. |
Summary of ChangesHello @ojeda-e, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request continues the codebase modernization effort by updating type hints to leverage Python 3.10+ features. The primary goal is to enhance the readability and consistency of type annotations within dataclass arguments and recipe metadata structures, ensuring the code adheres to contemporary Python best practices for type safety and clarity. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request effectively modernizes the type hints across the codebase to align with Python 3.10+ standards, replacing Optional
, Union
, List
, and Dict
with their modern counterparts (|
, list
, dict
). This significantly improves code readability and maintainability. The changes are consistent and well-executed. I've added a couple of suggestions to further enhance type specificity in one of the files, building upon the great work already done.
75167f3
to
a1b90f3
Compare
Signed-off-by: ojeda-e <[email protected]>
a1b90f3
to
1599812
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution! This looks good to me, the places that were updated (e.g. changing TrainingArguments
return type to TrainingArguments | None
) all look appropriate
Thanks @brian-dellabetta, |
Thanks @ojeda-e , I'm looking into the CI issue, a new dependency version must be breaking things. Everything looks good in your changes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Thanks for the pr!
Signed-off-by: Brian Dellabetta <[email protected]>
a9fb590
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated to resolve some broken tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good
This PR complements the work in #1933 to modernize codebase with +3.10, now covering type hints:
make test
andmake style
passing.