Allow arbitrary template kwargs in processors#44881
Allow arbitrary template kwargs in processors#44881zucchini-nlp merged 9 commits intohuggingface:mainfrom
Conversation
|
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. |
| @@ -475,6 +472,7 @@ class CustomProcessorKwargs(ProcessingKwargs, total=False): | |||
|
|
|||
| class TokenizerChatTemplateKwargs(TypedDict, total=False): | |||
| """ | |||
| NOTE: `TokenizerChatTemplateKwargs` is deprecated and will be removed in future versions | |||
There was a problem hiding this comment.
I can't seem to find a proper way to deprecate a TypedDict when it's used only as typing hint 🥲
Rocketknight1
left a comment
Rocketknight1
left a comment
There was a problem hiding this comment.
Looks good with some things that felt a little weird to me - I'm not sure they're bugs, but it might be good to double-check them!
| logger.warning( | ||
| "Kwargs passed to `processor.__call__` have to be in `processor_kwargs` dict, not in `**kwargs`" | ||
| ) | ||
| processor_kwargs = processor_kwargs_from_kwargs |
There was a problem hiding this comment.
Should this be a dict merge and not a dict replacement? If processor_kwargs is not empty in this line then it gets clobbered.
There was a problem hiding this comment.
ahh right, good catch
There was a problem hiding this comment.
on a second thought, I don't want to allow passing both. For new releases, users need to pass only processor_kwargs, or following deprecated behavior pass simply as arbitrary **kwargs
There was a problem hiding this comment.
Some values have to be passed twice now, right? Like return_tensors and load_audio_from_video can be passed directly here as well as in processor_kwargs?
There was a problem hiding this comment.
I followed the tokenizer so we sync in API and split out return_tensors away from processing kwargs. And load_audio_from_video is not used by processor, it's only chat template arg for loading audios
|
[For maintainers] Suggested jobs to run (before merge) run-slow: aria, smolvlm, voxtral |
This reverts commit 2d46cc5.
* . * warn * only common tests * . * . * dont import deprecated typed dicts * print... * merge dicts if both are passed * Revert "merge dicts if both are passed" This reverts commit 2d46cc5.
4 participants
What does this PR do?
As per title, we don't need a weird way to filter out kwargs anymore because now we don't rely on
tokenizer.apply_chat_template. I didn't delete the unusedTypedDictyet and will deprecate for at least 3 minor releases, they might be used by some remote code modelsCredits to #44817 for
jinja2.metaTODO: make sure other models don't have the patch