-
-
Notifications
You must be signed in to change notification settings - Fork 140
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
ENH: use studentized range over statsmodels for pairwise tests #229
ENH: use studentized range over statsmodels for pairwise tests #229
Conversation
This is fantastic, thanks @swallan! I somehow missed that addition in SciPy. The unit tests are failing because of an unrelated issue (#227), but have you tried to run the test and doctest locally? Are there any modifications required? I am assuming that the overall impact on the resulting p-values is going to be so small that it might not even require updating the unit tests and examples in the docstring, but could you please check? Also, could you edit the changelog.rst file, under the 0.6.0.dev section? PS: read more about the contributing guidelines here. Thank you! |
@raphaelvallat I did run the Output of locally running tests: (all passing) ❯ pytest pingouin ================================================= test session starts ================================================= platform darwin -- Python 3.8.9, pytest-6.2.5, py-1.11.0, pluggy-1.0.0 rootdir: /Users/swallan/Documents/SciPy/scipy_meson/pingouin, configfile: setup.cfg collected 86 items
I didn't realize running the doctests before, but I have now updated the p-values. The changes needed were small. Here are the tests passing with that change (to be pushed). (all passing) ❯ pytest --doctest-modules ============================================================== test session starts ============================================================== platform darwin -- Python 3.8.9, pytest-6.2.5, py-1.11.0, pluggy-1.0.0 rootdir: /Users/swallan/Documents/SciPy/scipy_meson/pingouin, configfile: setup.cfg collected 165 items I also updated the changelog. |
Merging now, thanks again! |
This PR replaces use of statsmodels' studentized range distribution functions (
statsmodels.stats.libqsturng.qsturng
,statsmodels.stats.libqsturng.psturng
) with those from SciPy now thatscipy.stats.studentized_range
is available (SciPy 1.7.0+).Speed is maintained, and results are more accurate because SciPy's implementation evaluates the integral for the provided arguments rather than interpolating from tabulated results. statsmodels has adopted this change internally as well.
For example, this box and whiskey plot (from here) compares the relative accuracy of the studentized range CDF in SciPy, statsmodels, and R (ptukey) across a wide range of inputs.