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

pairwise_nonparametric() #209

Closed
Tracked by #242
michalkahle opened this issue Nov 10, 2021 · 5 comments · Fixed by #246
Closed
Tracked by #242

pairwise_nonparametric() #209

michalkahle opened this issue Nov 10, 2021 · 5 comments · Fixed by #246
Labels
docs/testing:book: Documentation and unit testing IMPORTANT❗

Comments

@michalkahle
Copy link
Contributor

Hi, I was looking for non-parametric pairwise tests and only found the parameter parametric=False of the pairwise_ttests() after some time from the flowcharts. This seems confusing to me. I'd suggest adding function pairwise_nonparametric() for this purpose mainly because of discoverability and clarity. Also pls note that the documentation to pairwise_ttests() just says: "Pairwise T-tests.". I'd be happy to prepare PR if that helps.

@raphaelvallat
Copy link
Owner

Hi @michalkahle,

Thanks for opening the issue. I am more in favor of improving the current documentation rather than creating a whole new function. The reason for that is that the pairwise_ttests function is one of the longest / more complicated function in Pingouin, and I think it would be messy to duplicate 99% of it just to eventually change one line of code related to which lower-level test we're using.

We might also think about a potential function renaming, e.g. pairwise_tests.

Thanks,
Raphael

@raphaelvallat raphaelvallat added the docs/testing:book: Documentation and unit testing label Nov 11, 2021
@michalkahle
Copy link
Contributor Author

I thought about creating a new function but just calling the old one with
parametric=False. But I like renaming it to pairwise_tests even better.
The pairwise_ttests would be retained but just give a deprecation warning
and call pairwise_tests. Documentation would be updated to reflect the
various tests that could be performed in pairwise fashion. What do you
think?

@raphaelvallat
Copy link
Owner

That sounds great!

@raphaelvallat
Copy link
Owner

To implement in the next major release (0.6.0)?

@michalkahle
Copy link
Contributor Author

Ok, working on the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs/testing:book: Documentation and unit testing IMPORTANT❗
Projects
None yet
2 participants