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

Spectral correlation density #14795

Closed
wants to merge 2 commits into from
Closed

Spectral correlation density #14795

wants to merge 2 commits into from

Conversation

shilpiprd
Copy link

ENH
I did some research and the following code seemed like an estimation of Spectral Correlation Density. I got this code by looking in one of the links mentioned above.Not sure if it's correct.
If it's not I'd like to know if we're expected to write the entire code by ourself because i can't find anything more relevant.
Please review this is my first pull request.

#14783
I'm sorry, it seems I've submitted the same file twice

@@ -0,0 +1,93 @@
function Coh = SCoh_W(y,x,alpha,nfft,Noverlap,Window,opt,P)
% Coh = SCoh_W(y,x,alpha,nfft,Noverlap,Window,opt,P)
% Welch's estimate of the Cyclic Spectral Coherence of signals y and x
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It says Welch's estimation here and apparently Welch's method is one of the ways to create an estimation.

@rkern
Copy link
Member

rkern commented Oct 1, 2021

It looks like you just checked in the MATLAB code from MATLAB Central. That should just be a reference for you as you write the Python implementation to submit as a PR. We don't need the MATLAB code to be checked in. I suggest starting with a fresh branch (rather than git rming the MATLAB file and adding more commits to this one).

@tupui
Copy link
Member

tupui commented Oct 2, 2021

Hi @shilpiprd, thanks for this first draft. Aside from what Robert said, don't forget to add the license in a comment. Better solution would be to contact the original author and ask explicitly for use in SciPy under our BSD-3-clause.

https://www.mathworks.com/matlabcentral/fileexchange/48909-cyclic-spectral-analysis?s_tid=srchtitle#license_modal

@tupui tupui marked this pull request as draft October 2, 2021 09:36
@tupui tupui added enhancement A new feature or improvement scipy.signal labels Oct 2, 2021
@tupui
Copy link
Member

tupui commented Feb 3, 2022

Thank you again @shilpiprd. Since there was no progress here and lot of people seem interested, I will close this one in favour of #15519.

My message on the issue:

Thank you for the new PR @qbarthelemy. Since @shilpiprd did not reply for a while either here and on the PR it's ok but we generally don't like to have competing PRs. I will close your PR @shilpiprd. You are all invited to help review/discuss the new PR #15519

@tupui tupui closed this Feb 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement A new feature or improvement scipy.signal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants