fix: prevent input side-effects in processor text args
Author: nph4rd
Pull request: yes

# What does this PR do?

Fixes input side-effects in multiple processor classes when the `text` input is a list.


Fixes https://github.com/huggingface/transformers/issues/36865


## Before submitting
- [ ] This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
- [x] Did you read the [contributor guideline](https://github.com/huggingface/transformers/blob/main/CONTRIBUTING.md#create-a-pull-request),
      Pull Request section?
- [ ] Was this discussed/approved via a Github issue or the [forum](https://discuss.huggingface.co/)? Please add a link
      to it if that's the case.
- [ ] Did you make sure to update the documentation with your changes? Here are the
      [documentation guidelines](https://github.com/huggingface/transformers/tree/main/docs), and
      [here are tips on formatting docstrings](https://github.com/huggingface/transformers/tree/main/docs#writing-source-documentation).
- [ ] Did you write any new necessary tests?


## Who can review?

@ArthurZucker  @qubvel 

Comments:
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 `Ready for review` button (at the bottom of the PR page).

For me it seems the problem is in how qwen2-vl is written. Other VLMs do not override the `text` and use save expanded prompts in a new list. So rewriting Qwen2-VL is more preferable than adding a `copy()` in all processors 

@zucchini-nlp - thanks for the comment! 😃 

Just to be clear: I added the `copy()` in **all** the processors that have a similar pattern like:

```python
for sample in text:
    sample = sample.replace(...)
```

This isn't **just** Qwen-based processors. One example: [Pixtral](https://github.com/huggingface/transformers/blob/6a2627918d84f25422b931507a8fb9146106ca20/src/transformers/models/pixtral/processing_pixtral.py#L205).

So, if I understand correctly, you're saying it'd be preferrable to change *all* these processors to have something like:

```python
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 `.copy()` to the rest of the VLM processors. That would imply less changed lines and better readability IMO -- e.g. no variable name change. However, if what you're saying is the consensus then I'd be happy to make the changes.

> That would imply less changed lines

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.

@nph4rd not a big difference for me, I am good whichever way as long as it doesn't do in-place modifications. I was surprised to see so many processor's changed, while the error is applicable only to Qwen VL 

Let's also see what @molbap thinks

@molbap any thoughts here? This is a pretty tricky bug when encountered downstream that is hurting the larger ecosystem

Sorry, qwen has been fixed already as part of a different PR. We might not need this fix anymore, other VLMs don't do in-place modification

@qubvel - apologies for the delay. I've just merged main.

Missed out on this - I think we should not mutate inputs indeed. 

I think it was already fixed as part of another PR already for qwen models, and other models don't mutate input

> I think it was already fixed as part of another PR already for qwen models, and other models don't mutate input

@zucchini-nlp - could you link the PR? I can't find it

I'm not sure about the other models. I don't see why they wouldn't. I tried to include all of the processors that had in-place mutations.

@nph4rd  it was https://github.com/huggingface/transformers/pull/37342 because some tests would fail otherwise (I think that test was modified right before merging though). And since the test passed for other models, I assume no other processor has in-place modifications. We usually save expanded inputs in a new list, and qwen was an more of an exception 😄 

If you see other models doing in-place modification, feel free to rebase and update the PR. Will be very much welcome. And adding a test would be nice so we catch these processors before merging

Ah, I see, @zucchini-nlp. Looks good! Thanks for the heads up.

In that case, @molbap, I'll close this PR and the corresponding issue 👍