-
Notifications
You must be signed in to change notification settings - Fork 87
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
[BUG] mpdist window length #1513
Conversation
Thank you for contributing to
|
could you make a change to run the CI again? If there are other problems would be good to create an issue. |
Overall I agree that having m=0 just feels weird as default value, None would be better ! At some point the same functionality should be available with similairty search if you want something "in house". Advantage would be that you could choose the internal distance to use (ED, DTW, etc...) |
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 to me now !
fixes #1498
the window length (called m for some reason) has as default value of 0, which means its set to a quarter of the length of the series. However, this check was happening after forming the windows, so it was trying to form windows with 0 length.
I think it might be better to just use stumpy for this, m = None should be the default and k/threshold should definitely be a parameter. I will see if it makes a difference in a separate PR