-
Notifications
You must be signed in to change notification settings - Fork 8
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
Sliding window changes #33
base: main
Are you sure you want to change the base?
Conversation
Thanks @mibur1 ! Overall it looks good! Thank you for making the changes. There a few comments below: |
@mibur1 lmk if you have any Qs! |
Co-authored-by: MOHAMMAD TORABI <[email protected]>
@mtorabi59 Thanks for the quick review and comments, I hope I addressed all of them. I will think about tests for the sliding window and add some if I have an idea. |
@mibur1 Thanks, everything looks good! There might only be some formatting changes by black or flake8. |
@mibur1 If you're familiar with pre-commit, it would be great to run it locally and make the required formatting changes, ow I'll fix them! |
Thanks! I'll try to do this as well as the unit tests next week. But please feel free to do it yourself if you would like to have it done earlier. |
Hi @mtorabi59
I implemented the changes as discussed in #31.
The changes are:
window_std
parameter to Sliding Window, Discrete HMM, and Sliding Window ClusteringNone
, which results to the original std of3 * W / 22
Feel free to modify this to your liking. Also I did not write any tests/documentation, please let me know if I should do so :)