Skip to content
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

Allow complex dtype of input #326

Closed
lamorton opened this issue Oct 6, 2017 · 2 comments · Fixed by #349
Closed

Allow complex dtype of input #326

lamorton opened this issue Oct 6, 2017 · 2 comments · Fixed by #349
Milestone

Comments

@lamorton
Copy link

lamorton commented Oct 6, 2017

Presently, the function 'cwt' does not operate on complex input data. It calls '_check_dtype,' which causes a downcast to float, discarding the imaginary part of the input data. It is relatively simple to add 'complex' to the list of allowed dtypes inside '_check_dtype'; I have done it on my local copy and it works just fine afterwards.

@grlee77
Copy link
Contributor

grlee77 commented Oct 9, 2017

Thanks for raising this issue. The discrete wavelet transform routines currently also use that _check_dtype utility and the C code for convolution does not currently support complex data types. The cwt routine does not have this problem as it uses np.convolve which supports complex data types. I think a reasonable solution would be to just make a _check_dtype_cwt that has the behavior you describe and leave the _check_dtype cython function as is for now.

Are you interested in making a PR for this? I don't think it would even be necessary to have _check_dtype_cwt be in a Cython file. It could just be a function within cwt.py itself.

That said, in PR #300 I did add C99 complex support to the C-code convolution routines and the complex types you mention are included within _check_dtype as part of that PR. That PR is mainly concerned with the discrete transform (to remove the need for calling the transforms on the real and imaginary parts of the data independently as in the lines here). That PR has been stalled for a bit pending a decision on how best to handle detection of C99 support (which is lacking in MSVC), but if merged would also resolve the issue raised in this PR.

@grlee77
Copy link
Contributor

grlee77 commented Feb 20, 2018

when #300 was merged, this issue was resolved as well.

I have opened #349 to add a test case for complex-valued CWT to make sure this continues to work into the future.

@grlee77 grlee77 closed this as completed Feb 20, 2018
@rgommers rgommers added this to the v1.0 milestone Feb 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants