-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
use PyWavelet for CWT #1097
use PyWavelet for CWT #1097
Conversation
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.
Thank you very much for taking the time to do the fix! I am ok with the changes. Did you have a chance to check the resulting features, are they the same?
Could you also fix the styling issue in the code? Thanks!
I have seen in the CI/CD that the pywavelets package in the version you were using is not available for python versions 3.9 and older. I can (and should!) drop the python versions 3.7 and 3.8, but 3.9 has not reached end of life yet, so I would like to keep it.
Is this particular version of pywavelets needed? Update: I did remove python 3.7 an 3.8 from the CI/CD an added 3.10-3.12. I added a limit for scipy to make the CI/CD succeed for now. |
@igor-pechersky - are the questions I noted above something you want to take a look at? Is this something you would have time to check in the near future? No worries if not, but I would probably take over the PR in this case to make sure we can release a fix. Thanks again! |
Taking over the PR. The cwt function is also present in older versions of pywavelets, so the high constraint is not needed. |
Fix #155, follow blue-yonder/tsfresh#1097 Signed-off-by: Johannes Mueller <johannes.mueller4@de.bosch.com>
#1096