-
Notifications
You must be signed in to change notification settings - Fork 881
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: replacing lambda with named function to make model pickable #1594
Conversation
Codecov ReportBase: 94.08% // Head: 94.04% // Decreases project coverage by
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more Additional details and impacted files@@ Coverage Diff @@
## master #1594 +/- ##
==========================================
- Coverage 94.08% 94.04% -0.04%
==========================================
Files 125 125
Lines 11171 11164 -7
==========================================
- Hits 10510 10499 -11
- Misses 661 665 +4
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
The pickle issue was also occurring for Noticed that the save/load cycle was not tested on all models, I don't know if we want to change this? I manually checked all the local models, it seems like FFT was the only one causing pickling issues anyway. |
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.
Looks good, thanks a lot!
Just added the Callable
for better interpretability
Co-authored-by: Dennis Bader <dennis.bader@gmx.ch>
…t8co#1594) * fix: replacing lambda with named function to make model pickable * fix: issue was also occurring with the exponential de-trending function * fix: adding typing * fix: linting --------- Co-authored-by: Dennis Bader <dennis.bader@gmx.ch>
Fixes #1587.
Summary
save()
was not working on FTT model if thetrend
argument was neither "poly" or "exp" because the functionlambda x : 0
is not pickable. Just created a class method to have a named function that can be pickled and updated the docstring.Other Information
I'll make sure all the other models
save()
method are working before converting this draft PR into a real one.