Skip to content

fix(model): replace mutable default evals_result=dict() with None across 27 models#2168

Open
warren618 wants to merge 1 commit intomicrosoft:mainfrom
warren618:fix/mutable-default-evals-result
Open

fix(model): replace mutable default evals_result=dict() with None across 27 models#2168
warren618 wants to merge 1 commit intomicrosoft:mainfrom
warren618:fix/mutable-default-evals-result

Conversation

@warren618
Copy link
Copy Markdown

Summary

Fixes #2167

27 models use evals_result=dict() as a mutable default argument in fit(). Changed to evals_result=None with explicit guard, matching the pattern in gbdt.py.

Changes

For each of the 27 affected files:

- def fit(self, dataset, evals_result=dict()):
+ def fit(self, dataset, evals_result=None):
+     if evals_result is None:
+         evals_result = {}

27 files changed, 81 insertions, 27 deletions.

…oss 27 models

Using dict() as a default argument is a well-known Python antipattern:
the dict is created once at function definition time and shared across
all calls. This can cause state leakage between separate fit() calls.

Changed all 27 affected models in qlib/contrib/model/ to use
evals_result=None with an explicit guard, matching the pattern already
used in gbdt.py and highfreq_gdbt_model.py.
@warren618
Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree

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.

Bug: mutable default argument evals_result=dict() in 27 model fit() methods

1 participant