Skip to content

fix: replace mutable default arguments with None - #1260

Closed
Mr-Neutr0n wants to merge 1 commit into
OpenGVLab:mainfrom
Mr-Neutr0n:fix/mutable-default-arguments
Closed

fix: replace mutable default arguments with None#1260
Mr-Neutr0n wants to merge 1 commit into
OpenGVLab:mainfrom
Mr-Neutr0n:fix/mutable-default-arguments

Conversation

@Mr-Neutr0n

Copy link
Copy Markdown

Summary

Using mutable default arguments (e.g. def f(history=[])) is a well-known Python anti-pattern (docs). The default object is evaluated once at function definition time and shared across all calls, so mutations in one call silently leak into subsequent calls.

This PR replaces every mutable default list with None and initializes a fresh list inside the function body.

Changes

File Function Parameter
internvl_chat/internvl/model/internlm2/modeling_internlm2.py InternLM2ForCausalLM.build_inputs history=[] -> history=None
internvl_chat/internvl/model/internlm2/modeling_internlm2.py InternLM2ForCausalLM.chat history=[] -> history=None
internvl_chat/internvl/model/internlm2/modeling_internlm2.py InternLM2ForCausalLM.stream_chat history=[] -> history=None
clip_benchmark/clip_benchmark/datasets/builder.py build_vtab_dataset classnames=[] -> classnames=None
internvl_chat/eval/mathvista/utilities.py contains_quantity_word special_keep_words=[] -> special_keep_words=None

Example of the bug

def chat(history=[]):
    history.append("new message")
    return history

chat()  # ['new message']
chat()  # ['new message', 'new message']  <-- unexpected!

Test plan

  • Verified no remaining =[] or ={} in function signatures across the codebase
  • Each guard clause (if x is None: x = []) is placed before the parameter's first use
  • No behavioral change for callers that already pass an explicit argument

Using mutable default arguments (e.g. `def f(x=[])`) is a well-known
Python anti-pattern because the default object is shared across all
calls, leading to subtle and hard-to-debug state leakage between
invocations.

Replace mutable default lists with `None` and initialize inside the
function body in:

- `InternLM2ForCausalLM.build_inputs` (history=[])
- `InternLM2ForCausalLM.chat` (history=[])
- `InternLM2ForCausalLM.stream_chat` (history=[])
- `build_vtab_dataset` (classnames=[])
- `contains_quantity_word` (special_keep_words=[])
@Mr-Neutr0n

Copy link
Copy Markdown
Author

Closing this one to tidy up my open pull requests.

It's been open around five months with no review activity, and I'd rather withdraw it than leave it sitting in your queue. Nothing needed from you, and no hard feelings at all.

If the fix is still wanted, this can be reopened, or I'm happy to redo it properly against current main. Apologies for the noise.

@Mr-Neutr0n Mr-Neutr0n closed this Jul 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant