homepage

There are two test_get_only methods. The patch provides a partial
fix, but removes the following two lines from the first method as the
execution of these lines fails:
 for f in (self.module.nlargest, self.module.nsmallest):
 self.assertRaises(TypeError, f, 2, GetOnly())
because heapq.nlargest is stuck in an infinite loop when the sequence
does not have a length. This seems to be a bug in nlargest. See the
following test that runs with 100 % cpu usage.
$ ./python
Python 3.4.0a2+ (default:f6792f734fcc, Sep 28 2013, 17:09:46) 
[GCC 4.3.2] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> class GetOnly:
... "Dummy sequence class defining __getitem__ but not __len__."
... def __getitem__(self, ndx):
... return 10
... 
>>> import heapq
>>> heapq.nlargest(2, GetOnly())
^CTraceback (most recent call last):
 File "<stdin>", line 1, in <module>
 File "Lib/heapq.py", line 455, in nlargest
 result = _nlargest(n, it)
 File "<stdin>", line 3, in __getitem__
KeyboardInterrupt
>>>

Good catch! Would like to hear from Raymond what the intent of these tests was - looks like "the real" test_get_only (which hasn't been executing) has multiple failures.

@Xavier de Gaye: Nice, you found many similar issues. How did you find them? Would it be possible to automate the detection of duplicated test names?

It was proposed, in issue 16056, to enhance `make patchcheck` with the
detection of duplicate code names. This triggered the creation of issue 16079.
The script named duplicate_code_names_2.py, in issue 16079, listed about 20
duplicate names among all the non-nested functions, classes or methods in the
whole repository (default branch), mostly duplicated test names. I have
entered an issue for each one of these cases, they are listed in issue 16079.
Yes, this detection can be automated.

I have played around with this test.
The major issue (there are other issues as well but not so difficult) is whether nlargest and nsmallest should support iterator that could be endless iterator or reject it (by checking __len__ attribute) straight away.
Once we decide that, the fix should be not that hard.
For example, I want to get 2 largest numbers from fibonacci number series.
heapq.nlargest(2, fibonacci_iterator) => what is the answer for this?
Exception or let it be stuck forever.

> The major issue (there are other issues as well but not so difficult) is whether nlargest and nsmallest should support iterator that could be endless iterator or reject it (by checking __len__ attribute) straight away.
Well, failing with an exception isn't right. I can imagine doing (and probably have done, at some point) nlargest on any number of generators, streams of data, etc.
So unless anyone can solve the halting problem (:-)) the behavior of nlargest is correct as possible; nlargest(2, GetOnly()) behaves just like list(GetOnly()) - they both hang forever.
The proper action here is just a fix in the tests: make GetOnly raise IndexError at some point so that it is a proper sequence. (also rename the second test to get_cmp_only)

Attached patch fixes the tests.

Any thoughts/reviews on the patch?

In the 2.7 branch, the test was removed in issue 7871 (changeset https://hg.python.org/cpython/rev/0130e574f5be).

That is one approach, of course, but imo pretty incomplete. A test that is named "test_get_only" presumably would be expected to test __getitem__, not __cmp__. Confusingly there is still a GetOnly item declared in the module, but it is not used. Compare to test_len_only, which has a paired LenOnly class and test name.
Either GetOnly should be completely excised from the module and the test renamed to test_cmp_only (alternately test_cmp_err), or the GetOnly class should be fixed as in my patch.

New changeset 4101181fd87c2fab6456663d3c8cc99377cf0463 by Raymond Hettinger in branch 'master':
bpo-19119: Remove invalid test and rename a misnamed test (GH-15442)
https://github.com/python/cpython/commit/4101181fd87c2fab6456663d3c8cc99377cf0463

New changeset ef3ccd737020a0bb49ea0bfe48069f89bb5370a1 by Raymond Hettinger (Miss Islington (bot)) in branch '3.8':
bpo-19119: Remove invalid test and rename a misnamed test (GH-15442) (GH-15447)
https://github.com/python/cpython/commit/ef3ccd737020a0bb49ea0bfe48069f89bb5370a1

Removed the bogus test -- it was a cut-and-paste error from a scratch file.