-
Notifications
You must be signed in to change notification settings - Fork 181
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
[ENH] Piecewise Linear Approximation (PLA) #1807
Conversation
Thank you for contributing to
|
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.
couple very minor things, but looks good to me. Could you change the title of the PR, this is just PLA not catch22
might be worth looking at numba at some point? |
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.
A few comments. I did not notice initially that _fit
should not exist due to the tag setting, unless you think it is the tag which is wrong and the method should exist.
I am going to turn on the code coverage tests to its easier to see what lines are covered by unit tests.
…hanged tests to be more accuracte, and added other SWAB checks such as checking the use of sliding window.
Could you plot the test series and each of the options similar to the image above? We have utilities in the visualisation module which can do this. |
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.
LGTM. Any comments @TonyBagnall?
What does this implement/fix? Explain your changes.
Implements #948. Added tests for new series transformer PLA.
Does your contribution introduce a new dependency? If yes, which one?
No new dependency