DEP: add deprecation warning for binom_test #16307

deadb0d4 · 2022-05-29T13:22:18Z

Reference issue

Closes #16256.

What does this implement/fix?

stats.binom_test was deprecated in documentation only. This PR adds a deprecation warning and test.

Additional information

deadb0d4 · 2022-05-29T13:23:05Z

Hey @h-vetinari, could you please take a look at this PR?

scipy/stats/tests/test_morestats.py

scipy/stats/_morestats.py

deadb0d4 · 2022-05-29T22:59:10Z

I see the following error in the build. Is it something simple? Do I need something like this to silence this?

copying static files... done
copying extra files... done
dumping search index in English (code: en)... done
dumping object inventory... done
build finished with problems, 1 warning.
make: *** [Makefile:117: html-scipyorg] Error 1
make: Leaving directory '/home/circleci/repo/doc'
Traceback (most recent call last):
  File "dev.py", line 661, in <module>
    main(argv=sys.argv[1:])
  File "dev.py", line 268, in main
    subprocess.run(cmd, check=True)
  File "/home/circleci/.pyenv/versions/3.8.13/lib/python3.8/subprocess.py", line 516, in run
    raise CalledProcessError(retcode, process.args,
subprocess.CalledProcessError: Command '['make', '-Cdoc', 'PYTHON="/home/circleci/.pyenv/versions/3.8.13/bin/python"', 'html-scipyorg', 'SPHINXOPTS="-j2"']' returned non-zero exit status 2.

Exited with code exit status 1

scipy/stats/_morestats.py

tupui

Thanks @deadb0d4 for the PR. I have a suggestion.
The CI failures are unrelated and was fixed on main recently. Can you merge main to your branch please?

scipy/stats/_morestats.py

deadb0d4 · 2022-05-30T08:51:07Z

I only get this error python runtests.py --doc:

WARNING: autodoc: failed to import data 'multivariate_beta' from module 'scipy.stats'; the following exception was raised:
Traceback (most recent call last):
  File "/home/ubuntu/anaconda3/envs/scipy-dev/lib/python3.10/site-packages/sphinx/util/inspect.py", line 440, in safe_getattr
    return getattr(obj, name, *defargs)
AttributeError: module 'scipy.stats' has no attribute 'multivariate_beta'

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/home/ubuntu/anaconda3/envs/scipy-dev/lib/python3.10/site-packages/sphinx/ext/autodoc/importer.py", line 102, in import_object
    obj = attrgetter(obj, mangled_name)
  File "/home/ubuntu/anaconda3/envs/scipy-dev/lib/python3.10/site-packages/sphinx/ext/autodoc/__init__.py", line 327, in get_attr
    return autodoc_attrgetter(self.env.app, obj, name, *defargs)
  File "/home/ubuntu/anaconda3/envs/scipy-dev/lib/python3.10/site-packages/sphinx/ext/autodoc/__init__.py", line 2827, in autodoc_attrgetter
    return safe_getattr(obj, name, *defargs)
  File "/home/ubuntu/anaconda3/envs/scipy-dev/lib/python3.10/site-packages/sphinx/util/inspect.py", line 456, in safe_getattr
    raise AttributeError(name) from exc
AttributeError: multivariate_beta

looking for now-outdated files... none found
pickling environment... done
checking consistency... /home/ubuntu/scipy/doc/source/reference/generated/scipy.stats.multivariate_beta.rst: WARNING: document isn't included in any toctree

And with python runtests.py --refguide-check, I get this one:

/home/ubuntu/scipy/tools/../doc/source/tutorial/csgraph.rst
-----------------------------------------------------------

File "doc/source/tutorial/csgraph.rst", line 38, in /home/ubuntu/scipy/tools/../doc/source/tutorial/csgraph.rst
Failed example:
    word_list = open('/usr/share/dict/words').readlines()
Exception raised:
    Traceback (most recent call last):
      File "/home/ubuntu/anaconda3/envs/scipy-dev/lib/python3.10/doctest.py", line 1346, in __run
        exec(compile(example.source, filename, "single",
      File "<doctest /home/ubuntu/scipy/tools/../doc/source/tutorial/csgraph.rst[0]>", line 1, in <module>
        word_list = open('/usr/share/dict/words').readlines()
    FileNotFoundError: [Errno 2] No such file or directory: '/usr/share/dict/words'

tupui · 2022-05-30T08:54:26Z

You probably have some caching here. multivariate_beta was added and removed recently (yesterday) from main.

deadb0d4 · 2022-05-30T12:27:34Z

Weird HTTP error on check...

++ python tools/openblas_support.py
Could not download "https://anaconda.org/multibuild-wheels-staging/openblas-libs/v0.3.18/download/openblas-v0.3.18-macosx_10_9_x86_64-gf_1becaaa.tar.gz"
Traceback (most recent call last):
  File "tools/openblas_support.py", line 339, in <module>
    print(setup_openblas())
  File "tools/openblas_support.py", line 126, in setup_openblas
    typ = download_openblas(tmp, plat, ilp64)
  File "tools/openblas_support.py", line 96, in download_openblas
    response = urlopen(req)
  File "/Users/runner/hostedtoolcache/Python/3.8.12/x64/lib/python3.8/urllib/request.py", line [22](https://github.com/scipy/scipy/runs/6654554726?check_suite_focus=true#step:5:23)2, in urlopen
    return opener.open(url, data, timeout)
  File "/Users/runner/hostedtoolcache/Python/3.8.12/x64/lib/python3.8/urllib/request.py", line 5[31](https://github.com/scipy/scipy/runs/6654554726?check_suite_focus=true#step:5:32), in open
    response = meth(req, response)
  File "/Users/runner/hostedtoolcache/Python/3.8.12/x64/lib/python3.8/urllib/request.py", line 640, in http_response
    response = self.parent.error(
  File "/Users/runner/hostedtoolcache/Python/3.8.12/x64/lib/python3.8/urllib/request.py", line 569, in error
    return self._call_chain(*args)
  File "/Users/runner/hostedtoolcache/Python/3.8.12/x64/lib/python3.8/urllib/request.py", line 502, in _call_chain
    result = func(*args)
  File "/Users/runner/hostedtoolcache/Python/3.8.12/x64/lib/python3.8/urllib/request.py", line 649, in http_error_default
    raise HTTPError(req.full_url, code, msg, hdrs, fp)
urllib.error.HTTPError: HTTP Error [40](https://github.com/scipy/scipy/runs/6654554726?check_suite_focus=true#step:5:41)3: Forbidden
+ target=

tupui · 2022-05-30T12:28:29Z

It's fine. Not linked to this PR and intermittent on main.

tupui · 2022-05-30T13:34:25Z

@deadb0d4 @h-vetinari since it was previously asked to only doc-deprecate, could you send an email to the mailing list before we get this in?

deadb0d4 · 2022-05-30T14:36:49Z

I've sent an email to scipy-dev@python.org. Since I am the first time contributor and new subscriber, I believe it needs a verification.

h-vetinari

I think we should switch most of the tests to binomtest directly

h-vetinari · 2022-05-30T14:15:58Z

scipy/stats/tests/test_stats.py

+    with np.testing.suppress_warnings() as sup:
+        sup.filter(DeprecationWarning, "'binom_test' is deprecated")
+        for p, res in zip(pp, results):
+            assert_approx_equal(stats.binom_test(x, n, p), res,
+                                significant=12, err_msg='fail forp=%f' % p)
+            assert_approx_equal(stats.binom_test(50, 100, 0.1),
+                                5.8320387857343647e-024,
+                                significant=12)


Instead of adding a deprecation warning, can you please just switch to using binomtest here?

Hey @h-vetinari, with your comment below, do I get you correctly that we should only check binom_test in the below and switch to binomtest everywhere else?

Yes. Let's use the non-deprecated everywhere, except where we're testing the deprecation (and equivalence with the new method) explicitly.

h-vetinari · 2022-05-30T15:32:25Z

scipy/stats/tests/test_stats.py

+def test_binom_test_deprecation():
+    deprecation_msg = ("'binom_test' is deprecated in favour of"
+                       " 'binomtest' and will be removed in Scipy 1.11.0.")
+    with pytest.warns(DeprecationWarning, match=deprecation_msg):
+        stats.binom_test([1, 1])


In fact, this is essentially the only test we need for the deprecated binom_test, though I think we should extend it to also check equality with the result of binomtest. Something like:

@pytest.mark.parametrize('alternative', ['two-sided', 'greater', 'less']) def test_binom_test_deprecation(alternative): num = 10 X = np.random.randint(10, 100, (num,)) # needs N >= X N = X + np.random.randint(0, 100, (num,)) P = np.random.uniform(0, 1, (num,)) deprecation_msg = ("'binom_test' is deprecated in favour of" " 'binomtest' and will be removed in Scipy 1.11.0.") for x, n, p in zip(X, N, P): with pytest.warns(DeprecationWarning, match=deprecation_msg): res = stats.binom_test(x, n, p, alternative=alternative) assert res = stats.binomtest(x, n, p, alternative=alternative)

tupui

A heads up. The branching to 1.9 was done yesterday. So unless there is a compelling reason to make the work to backport this to the maintenance branch, we might have to update the deprecation version to 1.12.

Carreau · 2022-05-31T09:03:25Z

scipy/stats/_morestats.py

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


+@_deprecated("'binom_test' is deprecated in favour of"
+             " 'binomtest' and will be removed in Scipy 1.11.0.")


Personally when reading deprecation message, the critical information is since when the alternative is present – not when the current one will be removed. From my understanding this means binomtest is available since 0.19.0, so I would state that in the deprecation message as well.

Knowing since when it is available is often the difference between understanding whether you can do a blind rename, of have to do conditional based on version number.

Just my 2c.

@Carreau does the new message look good to you?

Add a missing whitespace Co-authored-by: Pamphile Roy <roy.pamphile@gmail.com>

deadb0d4 · 2022-05-31T16:21:10Z

Updated tests & warning message. Could you please take a look?

scipy/stats/tests/test_stats.py

Co-authored-by: Pamphile Roy <roy.pamphile@gmail.com>

scipy/stats/tests/test_stats.py

scipy/stats/_morestats.py

scipy/stats/tests/test_stats.py

scipy/stats/_morestats.py

scipy/stats/tests/test_stats.py

Co-authored-by: Pamphile Roy <roy.pamphile@gmail.com>

tupui

LGTM, let's wait an extra week to give time for people since you sent something to the mailing list.

deadb0d4 · 2022-06-10T09:08:04Z

Hey @tupui,

Can we merge this PR now?

tupui · 2022-06-10T11:56:53Z

Yes it's been a week now. Thank you again @deadb0d4 for the PR and everyone else for the reviews 😃

deadb0d4 changed the title ~~add deprecation warning for binom_test~~ DEP: add deprecation warning for binom_test May 29, 2022

j-bowhay reviewed May 29, 2022

View reviewed changes

scipy/stats/tests/test_morestats.py Show resolved Hide resolved

tupui reviewed May 29, 2022

View reviewed changes

scipy/stats/_morestats.py Outdated Show resolved Hide resolved

tupui added deprecated Items related to behavior that has been deprecated scipy.stats labels May 29, 2022

j-bowhay reviewed May 30, 2022

View reviewed changes

scipy/stats/_morestats.py Outdated Show resolved Hide resolved

tupui requested changes May 30, 2022

View reviewed changes

scipy/stats/_morestats.py Outdated Show resolved Hide resolved

h-vetinari reviewed May 30, 2022

View reviewed changes

tupui reviewed May 31, 2022

View reviewed changes

Carreau reviewed May 31, 2022

View reviewed changes

Dmitry Ulyumdzhiev and others added 4 commits May 31, 2022 16:00

do deprecate binom_test

ee00013

review: use _deprecated & keep tests for binom_test

d3f9032

update scipy/stats/_morestats.py

4d096e5

Add a missing whitespace Co-authored-by: Pamphile Roy <roy.pamphile@gmail.com>

add deprecated tag to docstring

6f531ab

Review: update warning msg & tests

1c11bcb

tupui reviewed May 31, 2022

View reviewed changes

scipy/stats/tests/test_stats.py Outdated Show resolved Hide resolved

Update scipy/stats/tests/test_stats.py

55adfc9

Co-authored-by: Pamphile Roy <roy.pamphile@gmail.com>

tupui reviewed May 31, 2022

View reviewed changes

scipy/stats/tests/test_stats.py Outdated Show resolved Hide resolved

tupui requested a review from h-vetinari June 1, 2022 08:22

tupui added this to the 1.10.0 milestone Jun 1, 2022

h-vetinari reviewed Jun 2, 2022

View reviewed changes

scipy/stats/_morestats.py Outdated Show resolved Hide resolved

h-vetinari reviewed Jun 2, 2022

View reviewed changes

scipy/stats/tests/test_stats.py Show resolved Hide resolved

tupui reviewed Jun 2, 2022

View reviewed changes

scipy/stats/_morestats.py Show resolved Hide resolved

scipy/stats/tests/test_stats.py Show resolved Hide resolved

Update scipy/stats/tests/test_stats.py

163fe3d

Co-authored-by: Pamphile Roy <roy.pamphile@gmail.com>

deadb0d4 requested a review from tupui June 3, 2022 10:05

tupui approved these changes Jun 3, 2022

View reviewed changes

elchappo approved these changes Jun 6, 2022

View reviewed changes

tupui merged commit 329e162 into scipy:main Jun 10, 2022

h-vetinari mentioned this pull request Jun 13, 2022

META: deprecation tracker #15765

Open

36 tasks

h-vetinari mentioned this pull request May 30, 2023

DEP: removal of stats.binom_test #18581

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DEP: add deprecation warning for binom_test #16307

DEP: add deprecation warning for binom_test #16307

deadb0d4 commented May 29, 2022

deadb0d4 commented May 29, 2022

deadb0d4 commented May 29, 2022 •

edited

tupui left a comment •

edited

deadb0d4 commented May 30, 2022

tupui commented May 30, 2022

deadb0d4 commented May 30, 2022

tupui commented May 30, 2022

tupui commented May 30, 2022

deadb0d4 commented May 30, 2022

h-vetinari left a comment

h-vetinari May 30, 2022

deadb0d4 May 30, 2022 •

edited

h-vetinari May 31, 2022

h-vetinari May 30, 2022

tupui left a comment

Carreau May 31, 2022

tupui Jun 1, 2022

deadb0d4 commented May 31, 2022

tupui left a comment

deadb0d4 commented Jun 10, 2022

tupui commented Jun 10, 2022

DEP: add deprecation warning for binom_test #16307

DEP: add deprecation warning for binom_test #16307

Conversation

deadb0d4 commented May 29, 2022

Reference issue

What does this implement/fix?

Additional information

deadb0d4 commented May 29, 2022

deadb0d4 commented May 29, 2022 • edited

tupui left a comment • edited

Choose a reason for hiding this comment

deadb0d4 commented May 30, 2022

tupui commented May 30, 2022

deadb0d4 commented May 30, 2022

tupui commented May 30, 2022

tupui commented May 30, 2022

deadb0d4 commented May 30, 2022

h-vetinari left a comment

Choose a reason for hiding this comment

h-vetinari May 30, 2022

Choose a reason for hiding this comment

deadb0d4 May 30, 2022 • edited

Choose a reason for hiding this comment

h-vetinari May 31, 2022

Choose a reason for hiding this comment

h-vetinari May 30, 2022

Choose a reason for hiding this comment

tupui left a comment

Choose a reason for hiding this comment

Carreau May 31, 2022

Choose a reason for hiding this comment

tupui Jun 1, 2022

Choose a reason for hiding this comment

deadb0d4 commented May 31, 2022

tupui left a comment

Choose a reason for hiding this comment

deadb0d4 commented Jun 10, 2022

tupui commented Jun 10, 2022

deadb0d4 commented May 29, 2022 •

edited

tupui left a comment •

edited

deadb0d4 May 30, 2022 •

edited