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

Add paired t-test greater and less than comparisons #85

Open
wants to merge 4 commits into
base: dev
Choose a base branch
from

Conversation

Magnushhoie
Copy link

Add t-test_paired-gt and t-test_paired-ls to StatTest. Currently only the two-sided alternative of paired t-test is available, while e.g. Mann-Whitney provides all three.

Follows the same naming scheme and passes automated tests and manual testing.

@trevismd trevismd changed the base branch from master to dev October 18, 2022 18:06
Copy link
Owner

@trevismd trevismd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @Magnushhoie ,
Thanks for the PR!
I guess it is convenient indeed to have these basic configurations included even though it is already possible to override the alternative parameter using stats_params.

Would you be willing to add basic tests ensuring their coverage in case something is updated later on (by us or scipy)?

Comment on lines 114 to 121

't-test_paired-gt': StatTest(stats.ttest_rel,
't-test paired samples', 't-test_rel', 't',
alternative="greater"),

't-test_paired-ls': StatTest(stats.ttest_rel,
't-test paired samples', 't-test_rel', 't',
alternative="less"),
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you move these below the two-sided counterpart and fix the alignment of the 2nd and 3rd lines?

Comment on lines +10 to +11
IMPLEMENTED_TESTS = ['t-test_ind', 't-test_welch',
't-test_paired', 't-test_paired-gt', 't-test_paired-ls',
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
IMPLEMENTED_TESTS = ['t-test_ind', 't-test_welch',
't-test_paired', 't-test_paired-gt', 't-test_paired-ls',
IMPLEMENTED_TESTS = ['t-test_ind', 't-test_welch',
't-test_paired', 't-test_paired-gt', 't-test_paired-ls',

@trevismd
Copy link
Owner

Oh, and perhaps it's worth doing that for independent samples as well, then?

@Magnushhoie
Copy link
Author

Hi Trevis,

Sure, tried to add some basic tests for matching p-values. I am struggling to figure out how the TestStatTest tests in tests/test_annotation.py are actually run. I hope the logic will work like it does when I coded it manually.

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

Successfully merging this pull request may close these issues.

None yet

2 participants