classification
Title: statistics.geometric_mean has no tests. Defer to 3.7?
Type: behavior Stage: resolved
Components: Versions: Python 3.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: steven.daprano Nosy List: mark.dickinson, methane, ned.deily, steven.daprano
Priority: normal Keywords: patch

Created on 2016-10-01 12:33 by mark.dickinson, last changed 2019-02-21 06:25 by methane. This issue is now closed.

Files
File name Uploaded Description Edit
geometric_mean_tests.patch steven.daprano, 2016-10-02 05:12 review
geometric_mean.patch methane, 2016-10-03 11:09 review
Messages (10)
msg277811 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2016-10-01 12:33
The newly-added statistics.geometric_mean function appears to have no tests at all, with the exception of a single doctest:

    >>> geometric_mean([1.10, 0.95, 1.12])
    1.0538483123382172

Steve, Ned: what do you think about taking geometric_mean out of Python 3.6, leaving us time to get it into shape for 3.7? The implementation of geometric_mean is complicated, with many edge cases. The original code was committed without review in #27181, and it currently suffers from some serious bugs: see #27761, #28111, #28327 (the first of these affects only the tests for nroot; the second and third are behaviour bugs in geometric_mean). With the lack of tests, and Python 3.6 b2 looming, I think the best thing to do might be to defer the inclusion of geometric_mean to Python 3.7.

I'll mark this as a release blocker until we decide what to do.
msg277812 - (view) Author: Steven D'Aprano (steven.daprano) * (Python committer) Date: 2016-10-01 12:50
Unfortunately I'm in a pickle at the moment, I cannot build 3.6 on any 
of the machines I have available (three), and no time to upgrade them to 
something that will build 3.6. But I can provide some tests, if 
somebody is willing to review and check them in to 3.6.
msg277837 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2016-10-02 00:56
I was hoping that the open issues could be resolved in time for b2 but that seems unlikely at this point.  So I have to agree that this feature should be deferred to 3.7.  Steven, can you make the necessary reverts on the 3.6 branch?
msg277851 - (view) Author: Steven D'Aprano (steven.daprano) * (Python committer) Date: 2016-10-02 05:12
> The newly-added statistics.geometric_mean function appears to have no 
> tests at all

That's weird and unfortunate. I certainly wrote tests, and I have a 
backup of them. I have no idea what happened.

Attached is a patch that adds the tests, but obviously I haven't been 
able to run this until I resolve my gcc issues and can build 3.6 again. 
(I did run this some weeks ago, and they passed *then*, but I cannot be 
sure they still pass now.) If somebody would like to test this for me, 
it would be appreciated.
msg277950 - (view) Author: Inada Naoki (methane) * (Python committer) Date: 2016-10-03 10:57
I run attached test, and saw following errors.

On macOS 10.11 (XCode 8)

$ hg summary
parent: 104258:0d948a46c59a
 test_invalid_sequences seems don't have to stay in CAPITest.
branch: 3.6
commit: 1 modified, 1 unknown
update: (current)


$ ./python.exe -m test.test_statistics -v
...
======================================================================
ERROR: test_negative_error (__main__.TestGeometricMean) (values=[-1])
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/inada-n/work/python/py36/Lib/test/test_statistics.py", line 1741, in test_negative_error
    self.assertRaises(exc, self.func, values)
  File "/Users/inada-n/work/python/py36/Lib/unittest/case.py", line 728, in assertRaises
    return context.handle('assertRaises', args, kwargs)
  File "/Users/inada-n/work/python/py36/Lib/unittest/case.py", line 177, in handle
    callable_obj(*args, **kwargs)
  File "/Users/inada-n/work/python/py36/Lib/statistics.py", line 567, in geometric_mean
    if isinstance(g, (numbers.Real, Decimal)):
NameError: name 'g' is not defined

======================================================================
ERROR: test_single_value (__main__.TestGeometricMean)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/inada-n/work/python/py36/Lib/test/test_statistics.py", line 1587, in test_single_value
    self.assertEqual(self.func([x]), x)
  File "/Users/inada-n/work/python/py36/Lib/statistics.py", line 567, in geometric_mean
    if isinstance(g, (numbers.Real, Decimal)):
NameError: name 'g' is not defined

======================================================================
ERROR: test_singleton_lists (__main__.TestGeometricMean)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/inada-n/work/python/py36/Lib/test/test_statistics.py", line 1762, in test_singleton_lists
    self.assertEqual(self.func([x]), x)
  File "/Users/inada-n/work/python/py36/Lib/statistics.py", line 567, in geometric_mean
    if isinstance(g, (numbers.Real, Decimal)):
NameError: name 'g' is not defined

======================================================================
FAIL: test_multiply_data_points (__main__.TestGeometricMean)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/inada-n/work/python/py36/Lib/test/test_statistics.py", line 1796, in test_multiply_data_points
    self.assertApproxEqual(self.func(data), expected, rel=1e-13)
  File "/Users/inada-n/work/python/py36/Lib/test/test_statistics.py", line 227, in assertApproxEqual
    check(first, second, tol, rel, msg)
  File "/Users/inada-n/work/python/py36/Lib/test/test_statistics.py", line 247, in _check_approx_num
    raise self.failureException(msg)
AssertionError:   6.27015835359916 != 695.9875772495068
  values differ by more than tol=0 and rel=1e-13
  -> absolute error = 689.7174188959076
  -> relative error = 0.990990990990991
msg277951 - (view) Author: Inada Naoki (methane) * (Python committer) Date: 2016-10-03 11:09
Attached patch fixes first two errors.
Last one error is from test.  I've commented it in review tool.
msg277996 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2016-10-03 19:36
One more geometric_mean issue (the current code can enter an infinite loop for Decimal inputs): #28351.

Steve, I really think we should postpone to 3.7. I'm very happy to help out with all the necessary fixes, tests and review, but I'm not going to have time for that before 3.6b2. If we defer, that takes the pressure off a bit and lets us get a good solid implementation in for 3.7.
msg278026 - (view) Author: Steven D'Aprano (steven.daprano) * (Python committer) Date: 2016-10-04 10:08
> Steve, I really think we should postpone to 3.7. [...]

If these fixes have to be in by the next beta (10th Oct), I fear that 
you are right. I can build up to changeset 103135:8b74e5528f35, but not 
beyond. I will be able to rectify that, but realistically it won't be 
before beta2.

I expect that harmonic_mean is okay to stay in though?

I feel frustrated at how close to complete geometric_mean is, 
disappointed that I can't work on it at the moment, and embarrassed that 
I'm letting the team down. Sorry for the extra work I've put you 
through.

I can do the removals -- probably tonight.
msg282604 - (view) Author: Inada Naoki (methane) * (Python committer) Date: 2016-12-07 09:19
3.6rc1 is released.
What is status of this issue?
msg282607 - (view) Author: Inada Naoki (methane) * (Python committer) Date: 2016-12-07 09:45
It had been rolled back.
https://hg.python.org/cpython/rev/9dce0e41bedd

Can we close this issue? #27181 is still open.
History
Date User Action Args
2019-02-21 06:25:11methanesetstatus: open -> closed
resolution: fixed
stage: test needed -> resolved
2016-12-07 09:45:22methanesetmessages: + msg282607
2016-12-07 09:19:31methanesetmessages: + msg282604
2016-10-10 17:30:15ned.deilysetpriority: release blocker -> normal
2016-10-04 16:32:00steven.dapranosetversions: - Python 3.6
2016-10-04 10:08:19steven.dapranosetmessages: + msg278026
2016-10-03 19:36:53mark.dickinsonsetmessages: + msg277996
2016-10-03 11:09:43methanesetfiles: + geometric_mean.patch

messages: + msg277951
2016-10-03 10:57:39methanesetnosy: + methane
messages: + msg277950
2016-10-02 05:12:38steven.dapranosetfiles: + geometric_mean_tests.patch
keywords: + patch
messages: + msg277851
2016-10-02 00:56:51ned.deilysetmessages: + msg277837
2016-10-01 12:50:20steven.dapranosetmessages: + msg277812
2016-10-01 12:33:54mark.dickinsoncreate