ENH: stats: Add `binomtest` to replace `binom_test`. #12603

WarrenWeckesser · 2020-07-22T20:53:49Z

Add a new function, binomtest, similar to binom_test but that returns an object with both the estimated proportion and the p-value as attributes, and that has a method to compute the confidence interval of the estimated proportion.

Also "doc-deprecate" binom_test.

WarrenWeckesser · 2020-07-22T21:19:24Z

binom_test in master returns a single floating point value, so we can't replace it with the bunch-like object that we recently implemented. So we have to figure out an API that allows the addition of the confidence interval calculation to the binomial test.

Instead of trying to modify the return value of binom_test, we could create a new function, say binomial_test, that returns an object with whatever attributes we want it to have. This is probably the simplest approach. It lets us implement the API of binomial_test without having to deal with backwards compatibility.

Alternatively, we could do what I've implemented in this PR, which is to change the return value of binom_test based on whether the argument confidence_level is None or not. (I'm sure the folks working on typing will love that!) If confidence_level is None (the default), the return value is just the p-value, for backwards compatibility. Otherwise, an instance of BinomTestResult is returned.

I'm not sure about this API. Ultimately, it might be better to create the new function binomial_test, and "doc-deprecate" binom_test. Then binomial_test could have the same default for confidence_interval (0.95) that we'll probably use in the other statistical test functions.

josef-pkt · 2020-07-22T21:28:24Z

There is no real code sharing between binom test and confint.

Alternative is to add confint as separate function (that's what I'm doing in statsmodels for now. All in one functions and classes are currently only for a few special cases.)

scipy/stats/_binom_test.py

scipy/stats/tests/test_morestats.py

scipy/stats/_binom_test.py

mdhaber · 2020-07-24T02:14:50Z

Making a new function to avoid tip-toeing around the old API makes sense to me.
Consider also binomtest as the name, without the underscore. The lack of underscore would be more consistent with the names of other tests (skewtest, kurtosistest, and normaltest), and binom is consistent with the abbreviation used elsewhere.

The new function, binomtest, returns an object with more information, and with a method to compute the confidence interval for the estimated proportion. The old version, binom_test, is deprecated.

WarrenWeckesser · 2020-11-13T09:07:59Z

I didn't comment when I updated this a couple weeks ago. I followed @mdhaber's suggestion to create a new function called binomtest.

The new function returns an object with more information, and with a method to compute the confidence interval for the estimated proportion. The old version, binom_test, is deprecated.

E.g.

In [63]: result = binomtest(4, n=31, p=0.25)                                                                                                          

In [64]: result                                                                                                                                       
Out[64]: BinomTestResult(k=4, n=31, alternative='two-sided', proportion_estimate=0.12903225806451613, pvalue=0.1471902718162708)

In [65]: result.pvalue                                                                                                                                
Out[65]: 0.1471902718162708

In [66]: result.proportion_estimate                                                                                                                   
Out[66]: 0.12903225806451613

In [67]: result.proportion_ci()                                                                                                                       
Out[67]: ConfidenceInterval(low=0.03630166197920805, high=0.29833582900779726)

The API probably needs a few tweaks, e.g. @mdhaber suggested using pi instead of p, to avoid confusion with the p in pvalue.

scipy/stats/_binomtest.py

scipy/stats/tests/test_morestats.py

scipy/stats/_binomtest.py

…_ci()

…refs.

rlucas7

This looks pretty good to me, I'll merge tomorrow unless @mdhaber merges beforehand.

scipy/_lib/_util.py

scipy/stats/tests/test_morestats.py

mdhaber · 2020-12-13T04:53:00Z

scipy/stats/morestats.py

@@ -2441,6 +2441,8 @@ def levene(*args, center='median', proportiontocut=0.05):
    return LeveneResult(W, pval)


+@np.deprecate(new_name='binomtest',
+              message='`binom_test` will be removed from SciPy 1.8.')


@rlucas7 @WarrenWeckesser Should this still be 1.8, given that this didn't make it into 1.6?
Also, did we send a message to the mailing list? I'm just thinking that we should do that before putting this in.

@mdhaber, I'll change that to 1.9. And I agree that this should be brought up on the mailing list before we merge this.

ok, I'll hold off merging then :)

The code was updated to say 1.9, and I just sent an email about this to the mailing list.

* Add `versionadded` markup to the Notes section of `binomtest`. * Bump removal version of `binom_test`. * Copyedit `_validate_int` docstring.

rlucas7 · 2020-12-13T18:52:13Z

@mdhaber I put a 1.7.0 milestone on this so that we don't forget

mdhaber · 2020-12-13T19:38:38Z

@mdhaber I put a 1.7.0 milestone on this so that we don't forget

I've been told to wait until merge.

WarrenWeckesser · 2020-12-13T20:12:51Z

As I noted in the mailing list, over in gh-12323, Ralf suggested that we just "doc-deprecate" binom_test. In my most recent commit I removed the deprecation of binom_test, and added a line to the docstring saying it is deprecated.

sethtroisi · 2020-12-14T21:33:34Z

scipy/_lib/tests/test__util.py

+    def test_validate_int(self):
+        n = _validate_int(4, 'n')
+        assert n == 4


I pseudo expected operator.index(Fraction(2, 1)) to work maybe also test operator.index(np.array(1))

The test looks at the type of the object, not the value. In general, not all Fraction objects are integers, just like not all float objects are integers. I added a few more tests, including one to test that a scalar array is acceptable, and another that a Fraction is not (even if the value of the Fraction is actually an integer).

mdhaber · 2020-12-18T07:41:01Z

scipy/_lib/tests/test__util.py

        assert n == 4

-    def test_validate_int_bad1(self):
+    @pytest.mark.parametrize('n', [4.0, np.array([4]), Fraction(4, 1)])


I guess I didn't notice that it was this strict. Some SciPy functions accept everything under the sun; this seems like a bit of a departure from the norm. Thoughts on why converting to an int internally and checking for equality with the original is not good enough? (This is Python, right?)

mdhaber · 2020-12-31T06:19:11Z

@WarrenWeckesser Time to merge this, no?

WarrenWeckesser · 2021-01-04T04:15:28Z

I've moved this from "draft" to "ready to review". The one test failure on TravisCI is the old test_symmetric_modes ARPACK failure, and is not related to this PR.

I sent an email to the mailing list about this PR on Dec. 13, which triggered no discussion (and therefore no objections!). So I think this is ready.

mdhaber · 2021-01-04T04:43:51Z

Thanks @WarrenWeckesser, @rlucas7!

WarrenWeckesser · 2021-01-04T04:56:00Z

Thanks all. I added a note to the SciPy 1.7.0 release notes on the wiki.

WarrenWeckesser marked this pull request as draft July 22, 2020 20:55

pvanmulbregt added the scipy.stats label Jul 23, 2020

WarrenWeckesser added the enhancement A new feature or improvement label Jul 23, 2020