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

ENH: use studentized range over statsmodels for pvalue #1

Closed
wants to merge 3 commits into from

Conversation

swallan
Copy link
Owner

@swallan swallan commented Feb 2, 2022

No description provided.

@mdhaber
Copy link

mdhaber commented Feb 3, 2022

Great. Do you know that's all of them? Do tests pass?

@@ -664,7 +664,7 @@ def pairwise_tukey(data=None, dv=None, between=None, effsize='hedges'):
# Critical values and p-values
# from statsmodels.stats.libqsturng import qsturng
Copy link

@mdhaber mdhaber Feb 3, 2022

Choose a reason for hiding this comment

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

Even if it's commented out, let's change it.
These would be all the places Dominic changed in raphaelvallatgh-156, but it's possible new uses have been added since then. Have you searched the repo for qsturng and psturng?

Copy link
Owner Author

Choose a reason for hiding this comment

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

I changed what dominic originally changed

Copy link

@mdhaber mdhaber Feb 3, 2022

Choose a reason for hiding this comment

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

OK, please do a search to double-check that nothing new has been added since then, and please change this commented-out stuff, too.
Then, if their tests pass, feel free to open up a PR to the main repo.
Thanks!

Copy link
Owner Author

@swallan swallan Feb 3, 2022

Choose a reason for hiding this comment

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

and yes, I did check for all uses. tests continue to pass

@swallan swallan force-pushed the use_scipy_studentized_range_over_statsmodels branch from e7b1e21 to 6443bd3 Compare February 3, 2022 03:55
@swallan
Copy link
Owner Author

swallan commented Feb 3, 2022

All tests pass, I don't see any other appearances of qstrung or psturng. for good measure I double checked that

  • studentized_range.sf -> psturng
  • studentized_range.ppf -> qstrung

@swallan swallan closed this Feb 12, 2022
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.

2 participants