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

BUG: Fix unequal varcorrection in tukeyhsd #9175

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

kuangzhexiang
Copy link

@kuangzhexiang kuangzhexiang commented Mar 22, 2024

  • tests added / passed.
  • code/documentation is well formatted.
  • properly formatted commit message. See
    NumPy's guide.

like tests in try_tukey_hsd.py
With the first dataset, results should be the same, as below.

image

@josef-pkt
Copy link
Member

Great, thanks for finding this mistake.

Do you have an example that uses this code path?
I don't remember now where this is used, and it does not have unit test coverage

@pep8speaks
Copy link

pep8speaks commented Mar 25, 2024

Hello @kuangzhexiang! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2024-04-07 02:49:39 UTC

@kuangzhexiang
Copy link
Author

Great, thanks for finding this mistake.

Do you have an example that uses this code path? I don't remember now where this is used, and it does not have unit test coverage

add a unit test in examples/try_tukey_hsd.py, is it ok?

@josef-pkt
Copy link
Member

No, unit tests need to go into a module that starts with tests

The most appropriate test file for Games-Howell unit test is "statsmodels\statsmodels\stats\tests\test_pairwise.py"

Also, the unit test need to go into a function with name starting with "test", see the other cases

The example files like examples/try_tukey_hsd.py were experimental and illustrative script files before the use of notebooks. We do not add those anymore.
Full examples go into docs notebooks.

If Games-Howell test verifies now against R, then we can add it to the official docs. I will look at that at the end of this PR.

If I understand correctly, then related PR and issue are #7332 and #8396

@kuangzhexiang
Copy link
Author

No, unit tests need to go into a module that starts with tests

The most appropriate test file for Games-Howell unit test is "statsmodels\statsmodels\stats\tests\test_pairwise.py"

Also, the unit test need to go into a function with name starting with "test", see the other cases

The example files like examples/try_tukey_hsd.py were experimental and illustrative script files before the use of notebooks. We do not add those anymore. Full examples go into docs notebooks.

If Games-Howell test verifies now against R, then we can add it to the official docs. I will look at that at the end of this PR.

If I understand correctly, then related PR and issue are #7332 and #8396

thanks a lot, added the unit test in test_pairwise.py

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

Successfully merging this pull request may close these issues.

None yet

3 participants