-
Notifications
You must be signed in to change notification settings - Fork 28.4k
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
fix: prevent input side-effects in processor text args #36866
base: main
Are you sure you want to change the base?
Conversation
Hi 👋, thank you for opening this pull request! The pull request is converted to draft by default. When it is ready for review, please click the |
For me it seems the problem is in how qwen2-vl is written. Other VLMs do not override the |
@zucchini-nlp - thanks for the comment! 😃 Just to be clear: I added the for sample in text:
sample = sample.replace(...) This isn't just Qwen-based processors. One example: Pixtral. So, if I understand correctly, you're saying it'd be preferrable to change all these processors to have something like: expanded_samples = []
for sample in text:
expanded_sample = sample.replace(...)
expanded_samples.append(expanded_sample) Correct? I must say: I don't find that cleaner 🤔 and if it's for the sake of consistency then I'd suggest adding the |
ha! 🤔 don't think that's true. My bad. Still, it seems a bit less readable to me. Again, happy to go either way if that's your suggestion. |
What does this PR do?
Fixes input side-effects in multiple processor classes when the
text
input is a list.Fixes #36865
Before submitting
Pull Request section?
to it if that's the case.
documentation guidelines, and
here are tips on formatting docstrings.
Who can review?
@ArthurZucker @qubvel