Skip to content

Conversation

@DavidALloyd
Copy link

Added ability to pass kwargs to downstream scipy functions using pairwise_tests. Previously did not have that ability, so changing things like the zero method or confidence weren't possible through pairwise_tests. pg.wilcoxon and pg.mwu already pass **kwargs downstream, so I simply modified pairwise_tests to accept **kwargs as a parameter and pass it as an argument when Wilcoxon, MWU, or ttest are called. Tested on example data and works without issue.

@codecov
Copy link

codecov bot commented Mar 21, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (5c5f61a) 98.55% compared to head (f21ee10) 98.55%.

❗ Current head f21ee10 differs from pull request most recent head a7a3698. Consider uploading reports for the commit a7a3698 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #352   +/-   ##
=======================================
  Coverage   98.55%   98.55%           
=======================================
  Files          19       19           
  Lines        3390     3390           
  Branches      559      559           
=======================================
  Hits         3341     3341           
  Misses         26       26           
  Partials       23       23           
Impacted Files Coverage Δ
pingouin/pairwise.py 99.46% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@raphaelvallat raphaelvallat self-requested a review April 16, 2023 07:22
@raphaelvallat raphaelvallat added the feature request 🚧 New feature or request label Apr 16, 2023
@raphaelvallat
Copy link
Owner

Thanks @DavidALloyd, great suggestion. I apologize about the delayed response.

Two things:

  1. The **kwargs should also be added to the recursive call

pt = pairwise_tests(

and when calculating the interaction:

for i, comb in enumerate(combs):

  1. Could you please add unit tests for mwu, wilcoxon, and ttest?

@DavidALloyd
Copy link
Author

DavidALloyd commented Apr 21, 2023

No worries @raphaelvallat, thanks for getting back! I can definitely add **kwargs to the other calls, but I have a bit of a dumb question though about the unit tests. I'm new to setting up tests like this, so my apologies if this is an ignorant question.

2. Could you please add unit tests for `mwu`, `wilcoxon`, and `ttest`?

test_nonparametryic.py already seems to contain tests for 'wilcoxon' and 'mwu'

def test_wilcoxon(self):

and there's a test for 'ttest' in test_parametric.py

def test_ttest(self):

Was there something specific you wanted me to implement in the testing of pairwise_tests in test_pairwise.py?

@raphaelvallat
Copy link
Owner

Hi @DavidALloyd!

My message was confusing, sorry about that. We want to add unit tests to test_pairwise_tests to ensure that the **kwargs work as expected. One simple way that doesn't require any ground-truth values from an external software (e..g JASP, JAMOVI, R, Matlab, etc) is to simply ensure that the output of pingouin.pairwise_tests is not the same when using a specific kwarg compared to the default. This needs to be tested for between/within and mixed design (with interaction). You can also just compare the output of pingouin.pairwise_tests (with only 2 groups) to the lower-level mwu, wilcoxon, and ttest functions when using a specific kwarg.

Does that make more sense?

Thanks!

@raphaelvallat
Copy link
Owner

Hi,

This PR has been inactive for several years so I'll go ahead and close it. Feel free to re-open.

Raphael

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature request 🚧 New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants