search

raamana / confounds

Conquering confounds and covariates: methods, library and guidance
https://raamana.github.io/confounds
Apache License 2.0
38 stars 14 forks source link

partial correlation and simple sns pair plot #13

Closed jameschapman19 closed 2 years ago

jameschapman19 commented 3 years ago

added in partial correlation and t test as in Dinga R, Schmaal L, Penninx BW, Veltman DJ, Marquand AF. Controlling for effects of confounding variables on machine learning predictions. BioRxiv. 2020 Jan 1.

There will definitely be mistakes in my process here. tests has a problem with test_estimator_API() but is nothing to do with anything I have done so not sure what the normal thing to do is as a contributor.

jameschapman19 commented 3 years ago

Hang on I need to update docs!

jameschapman19 commented 3 years ago

actually it's ok I did docstrings as I went I thought it would entail more than that

raamana commented 3 years ago

Thanks very much James for this PR. See my comments.

Also, would be converting / committing the script you showed us into a short tutorial? You can make a demo folder to add such scripts.. You know about Jupyter Notebooks right?

jameschapman19 commented 3 years ago

Responded to your comment, added the demo with cca, split out the functions in a different way. One of those where it could be refactored in so many ways but this way means we can calculate pairwise t-statistics and p-values for partial correlations and then the prediction/target partial correlation is then just a wrapper around them.

raamana commented 3 years ago

Thanks James. I will get to it as soon as possible while attending OHBM :)

raamana commented 3 years ago

Hi James, I noticed you added two more commits. I won't review them until you tell me you're done with making changes and it is ready for review. Please mention me here to ensure I won't miss that request.

ps: also I'd try change the commit message for each commit to reflect the actual changes. something short and simple

jameschapman19 commented 3 years ago

Ok changes done @raamana

raamana commented 3 years ago

hi @jameschapman19, if you dont feel another test is necessary (as I noted above), let me know, so we can discuss and I can merge it asap. Cheers :)

jameschapman19 commented 3 years ago

Just confirming I have seen this and will look in next couple of days

raamana commented 3 years ago

thanks James! take your time.

raamana commented 2 years ago

How goes James?

jameschapman19 commented 2 years ago

Just looking at this now/today

jameschapman19 commented 2 years ago

Think I have addressed your comments

raamana commented 2 years ago

Thanks James! happy holidays.

if you have other ideas or suggestions, please feel free to reach out anytime.