-
Notifications
You must be signed in to change notification settings - Fork 252
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix bug with version saving being overwritten on subsequent saves #481
Conversation
@@ -22,6 +22,7 @@ def default_meta() -> dict: | |||
"type": [], | |||
"explanations": [], | |||
"params": {}, | |||
"version": None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not strictly necessary here but added it as it serves as a kind of high-level "schema".
self.meta["name"] = self.__class__.__name__ | ||
self.meta["version"] = __version__ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The version needs to be added here instead of in the default_meta
because each explainer has it's own DEFAULT_META
defined in api.defaults
.
Codecov Report
@@ Coverage Diff @@
## master #481 +/- ##
==========================================
- Coverage 82.32% 82.32% -0.01%
==========================================
Files 76 76
Lines 10316 10315 -1
==========================================
- Hits 8493 8492 -1
Misses 1823 1823
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jklaise I just tested this and it seems to work nicely i.e. not overwriting with runtime version, but still raising a warning (which is preferable in my opinion) instead of an exception.
Fixes the issue found in SeldonIO/alibi-detect#236 (comment).