Processors: don't default padding side #33942
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. |
| value = ( | ||
| getattr(self.tokenizer, modality_key) | ||
| if hasattr(self.tokenizer, modality_key) | ||
| else tokenizer_init_kwargs[modality_key] | ||
| ) | ||
| default_kwargs[modality][modality_key] = value |
There was a problem hiding this comment.
I just realized the issue comes from this line when we use default tokenizer kwargs that were set during init. It a bit breaking if ppl change the attr like padding_side and that is not reflected in init_kwargs, so I think we can check first tokenizer attr and then fallback to the init kwargs
@molbap wdyt?
There was a problem hiding this comment.
Ah I see, yes that makes sense, it should be a "soft" check though to make sure we are not requiring anything at this stage, just that if it is present, it should be superseding.
Handling priorities in this script is 🤪 good catch though!
There was a problem hiding this comment.
yea, i think the check is soft as we first see if attribute if present, and if not fallback to the usual init_kwargs[key]
What does this PR do?
Fixes #33847. We should not have default values for padding side after the last PR that enabled passing
padding_sideto tokenizer directly. Users are free to set padding side as tokenizer attr or pass it to processors directlycc @yonigozlan if you had any PRs in progress with default padding-side 😉