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

Handle single-sample comparsion in pairwise_test #299

Merged
merged 4 commits into from
Sep 9, 2022

Conversation

George3d6
Copy link
Contributor

Due to a scipy interface change the pairwise_test can fail when obtaining the t-value from a single sample comparison, this, I believe, fixes that issue.

@raphaelvallat
Copy link
Owner

Hi @George3d6,

Thank you for this!

  1. Can you be more specific about which versions of SciPy have this potential bug? Did it appear in SciPy 1.9?
  2. Does this issue only occur for one-sample T-tests?
  3. There seems to be an unrelated unit test failure, I'll fix that now

@codecov
Copy link

codecov bot commented Aug 27, 2022

Codecov Report

Merging #299 (9fe378e) into master (dce908b) will decrease coverage by 0.04%.
The diff coverage is 94.59%.

@@            Coverage Diff             @@
##           master     #299      +/-   ##
==========================================
- Coverage   98.75%   98.71%   -0.05%     
==========================================
  Files          19       19              
  Lines        3298     3335      +37     
  Branches      529      537       +8     
==========================================
+ Hits         3257     3292      +35     
- Misses         24       25       +1     
- Partials       17       18       +1     
Impacted Files Coverage Δ
pingouin/multivariate.py 100.00% <ø> (ø)
pingouin/power.py 100.00% <ø> (ø)
pingouin/parametric.py 99.56% <33.33%> (-0.44%) ⬇️
pingouin/pairwise.py 99.46% <100.00%> (+0.05%) ⬆️

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

@George3d6
Copy link
Contributor Author

I'm actually unsure when the scipy interface might have changed, or if this was always the case, I'll look into it.
But as can bee seen, the current scipy interface is returning a tuple that the code treats as expecting to be a float or int, so the impl is most certainly wrong

@George3d6
Copy link
Contributor Author

@raphaelvallat as far as I can see this isn't a recent change in the scipy interface as I suspected, so I guess the bug was always there.

@raphaelvallat
Copy link
Owner

Got it, well I think we're good to merge. Thanks again for the PR!

@raphaelvallat raphaelvallat merged commit c66b685 into raphaelvallat:master Sep 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 💥 Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pairwise_tests producing array of t values internally
2 participants