Conversation
for more information, see https://pre-commit.ci
* Fix get_input_embeds call for VLMs * patch input_require_grads instead * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * cleanup old patch * cleanup old patch * cleanup * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Apply suggestion from @danielhanchen * use logger instead of prints * Move unsloth present set * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Daniel Han <danielhanchen@gmail.com>
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
* Silence fbgemm TMA print Also safer .push_to_hub * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
for more information, see https://pre-commit.ci
* login on token * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * cleanup old code * safer imports * cleanup * Return token after login * correct return types * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Apply suggestion from @danielhanchen * add back imports * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * finish return token --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Daniel Han <danielhanchen@gmail.com>
* Do not overwrite slots * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Summary of ChangesHello @danielhanchen, 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 significantly refactors the Hugging Face authentication process within the Unsloth library. By introducing a dedicated Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. 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.
Code Review
This pull request refactors the Hugging Face login logic into a new utility function hf_login in unsloth/models/_utils.py. This change centralizes the login process and removes duplicated code from several files, which is a good improvement for maintainability.
My review includes two suggestions:
- In
unsloth/import_fixes.py, I recommend re-adding__slots__to theHidePrintMessageclass for memory optimization. - In
unsloth/models/_utils.py, I suggest replacing a bareexcept:with a more specific exception to improve error handling robustness.
Overall, this is a positive refactoring with minor areas for improvement.
| @@ -72,8 +72,6 @@ def filter(self, x): | |||
|
|
|||
|
|
|||
| class HidePrintMessage: | |||
There was a problem hiding this comment.
The __slots__ attribute was removed from the HidePrintMessage class. Using __slots__ is a good practice for memory optimization, especially for classes that are instantiated many times. Since this class has a fixed set of attributes (_original_stream, _hidden_texts), reintroducing __slots__ would be beneficial for performance by preventing the creation of __dict__ for each instance.
class HidePrintMessage:
__slots__ = ("_original_stream", "_hidden_texts")| token = get_token() | ||
| if token is None: | ||
| return None | ||
| except: |
There was a problem hiding this comment.
Using a bare except: is generally discouraged as it can catch unexpected errors like SystemExit or KeyboardInterrupt, making the program harder to debug and control. It's better to catch a more specific exception, like Exception, or even more specific ones like ImportError or huggingface_hub.utils.HfHubError if applicable.
| except: | |
| except Exception: |
No description provided.