Repository navigation
Conversation
yashwantbezawada
force-pushed
the
fix/model-input-names-singleton-42024
branch
from
November 6, 2025 02:17
4d018bf to
6ba1ffb
Compare
Member
|
This solution LGTM but cc @ArthurZucker @itazap! |
This was referenced Apr 29, 2026
Collaborator
|
@itazap could you take a look when you get a chance? This is waiting on a maintainer: needs maintainer review (Rocketknight1 LGTM'd and cc'd ArthurZucker/itazap). |
Fixes huggingface#42024 The model_input_names attribute was defined as a class-level list, and when initializing tokenizer instances, they were all pointing to the same list object. This meant modifying model_input_names on one instance would affect all other instances. The issue was in tokenization_utils_base.py line 1417: ```python self.model_input_names = kwargs.pop("model_input_names", self.model_input_names) ``` When no model_input_names is passed in kwargs, it would use the class attribute directly (self.model_input_names), creating a reference to the shared list instead of creating a new list for the instance. Fixed by wrapping it in list() to ensure each instance gets its own copy: ```python self.model_input_names = list(kwargs.pop("model_input_names", self.model_input_names)) ``` This is a standard pattern for handling mutable default values in Python.
itazap
force-pushed
the
fix/model-input-names-singleton-42024
branch
from
November 3, 2026 22:15
6ba1ffb to
58ab3b2
Compare
itazap
approved these changes
Nov 3, 2026
itazap
left a comment
itazap
left a comment
Collaborator
There was a problem hiding this comment.
thank you and I apologize for the delay!
itazap
enabled auto-merge
November 3, 2026 22:16
|
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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Fixes #42024
I found this while testing tokenizers - when you modify
model_input_nameson one tokenizer instance, it was affecting all other instances of the same tokenizer class.The problem is that
model_input_namesis defined as a class-level list, and in the__init__method (line 1417), when no custommodel_input_namesis provided, it was just referencing the class attribute directly instead of making a copy.So all instances were sharing the same list object. This is a classic Python gotcha with mutable class attributes.
The fix is simple - wrap it in
list()to create a new list for each instance:Before:
After:
This ensures each tokenizer instance gets its own independent copy of the list. Now modifications to one instance won't affect others.
The reproduction from the issue shows the problem clearly - with the fix, the second tokenizer instance will have the original list values instead of inheriting the modifications from the first instance.