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

Making CorrelationTest nonparametric #236

Closed
wants to merge 1 commit into from

Conversation

ayushpatnaikgit
Copy link

The cor function performs pearson rank correlation, which assumes that the datapoints are normally distributed. The corspearman performs the spearman rank correlation test, which doesn't assume any underlying distribution. The latter is more robust and better suited for this package.

The cor function performs pearson rank correlation, which assumes that the datapoints are normally distributed. The corspearman performs the spearman rank correlation test, which doesn't assume any underlying distribution. The latter is more robust and better suited for this package.
@azev77
Copy link

azev77 commented Jun 6, 2021

Why not allow both options, w the default being ‘corspearman’?

@nalimilan
Copy link
Member

nalimilan commented Oct 2, 2021

Just like "correlation" (and therefore cor) refers to the Pearson correlation most of the time, "correlation test" is generally taken to refer to the Pearson correlation. Could you (as @azev suggested) add an argument allowing users to pass the correlation function they want? And then tests to check that the result is correct (checking against another implementation)?

EDIT: Maybe we'd better add a new SpearmanCorrelationTest instead, as we'll also want Kendall's tau correlation test too at some point, which is very different in terms of formula from the Pearson correlation test.

@nalimilan nalimilan mentioned this pull request Oct 2, 2021
@mapi1 mapi1 mentioned this pull request Jul 17, 2023
@nalimilan
Copy link
Member

Closing in favor of #304.

@nalimilan nalimilan closed this Sep 9, 2023
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 this pull request may close these issues.

None yet

3 participants