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

correlate_partial() #27

Merged
merged 5 commits into from
May 26, 2023
Merged

correlate_partial() #27

merged 5 commits into from
May 26, 2023

Conversation

LKobilke
Copy link
Collaborator

Implements the correlate_partial function based on ppcor::pcor.test.

Accepts exactly three numeric variables and computes pairwise partial correlations (pearson, kendall, spearman) for all combinations, i.e. pairs, of specified variables while controlling for the third variable. Stops and provides a case-specific warning if too little or too many variables or non-numeric variables are provided.
If input data is grouped, groups are dropped and a warning is issued.

Combinations of pairs are calculated using combinat::permn.

Returns a tibble.

Currently missing is the additional model as an attribute. This is added by @joon-e, see #24 .

@LKobilke LKobilke changed the title initial version of correlate_partial(...) alongside tests correlate_partial() Mar 16, 2023
@LKobilke LKobilke assigned LKobilke and unassigned MarHai and joon-e Mar 16, 2023
@LKobilke LKobilke requested review from joon-e and MarHai March 16, 2023 14:06
@MarHai
Copy link
Collaborator

MarHai commented Mar 16, 2023

Merge should aim at devel, not master.

@MarHai
Copy link
Collaborator

MarHai commented Mar 16, 2023

Code looks good and validates well.

However, I was expecting an extension to the correlate function with a parameter partial = TRUE. Why did you change this plan?

@LKobilke LKobilke changed the base branch from master to devel March 16, 2023 19:35
@LKobilke
Copy link
Collaborator Author

Code looks good and validates well.

However, I was expecting an extension to the correlate function with a parameter partial = TRUE. Why did you change this plan?

Good point. I've just pushed a new commit that implements a change to the correlate() function, i.e., adds the parameter partial = TRUE, and makes correlate_partial() an internal function. I was initially hesitant to do this since correlate() will grab all numeric variables if none are supplied. My concern was that this could lead to lots of information overload when reporting all possible x,y,z combinations. To avoid information overload correlate(partial = FALSE) will now work with two, three, or more variables, but correlate(partial = TRUE) will only accept three variables and throw a warning otherwise. This difference in behavior may seem inconsistent and I felt that a clear distinction between bivariate correlation and partial correlation might help to reduce this inconsistency.

Despite my initial concerns, I see the advantage to expand on the correlate() function. I'm open to keeping this version.

tests/testthat/test-correlate_partial.R Outdated Show resolved Hide resolved
tests/testthat/test-correlate_partial.R Outdated Show resolved Hide resolved
tests/testthat/test-correlate_partial.R Outdated Show resolved Hide resolved
tests/testthat/test-correlate_partial.R Outdated Show resolved Hide resolved
tests/testthat/test-correlate_partial.R Outdated Show resolved Hide resolved
tests/testthat/test-correlate_partial.R Outdated Show resolved Hide resolved
tests/testthat/test-correlate_partial.R Outdated Show resolved Hide resolved
tests/testthat/test-correlate_partial.R Outdated Show resolved Hide resolved
R/correlation.R Outdated Show resolved Hide resolved
@MarHai MarHai mentioned this pull request May 25, 2023
@MarHai MarHai merged commit 037133f into devel May 26, 2023
@MarHai MarHai deleted the correlate_partial branch May 26, 2023 09:01
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.

3 participants