Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

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

Open
warren618 wants to merge 1 commit intomicrosoft:mainmicrosoft/qlib:mainfrom
warren618:fix/mutable-default-evals-resultwarren618/qlib:fix/mutable-default-evals-resultCopy head branch name to clipboard
Open

fix(model): replace mutable default evals_result=dict() with None across 27 models#2168
warren618 wants to merge 1 commit intomicrosoft:mainmicrosoft/qlib:mainfrom
warren618:fix/mutable-default-evals-resultwarren618/qlib:fix/mutable-default-evals-resultCopy head branch name to clipboard

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

Morty Proxy This is a proxified and sanitized view of the page, visit original site.