Skip to content

fix: replace mutable default arguments with None#1260

Open
Mr-Neutr0n wants to merge 1 commit intoOpenGVLab:mainfrom
Mr-Neutr0n:fix/mutable-default-arguments
Open

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

Conversation

@Mr-Neutr0n
Copy link

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=[])
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