From f60f67269a861c3ff19dbee17342789a49a02cde Mon Sep 17 00:00:00 2001 From: Chase Sterling Date: Thu, 17 Oct 2019 21:16:56 -0400 Subject: [PATCH 1/5] Add test for min/max with generators. --- tests/test_future/test_builtins.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/test_future/test_builtins.py b/tests/test_future/test_builtins.py index d983f9d6..29f4c84a 100644 --- a/tests/test_future/test_builtins.py +++ b/tests/test_future/test_builtins.py @@ -1154,6 +1154,10 @@ def __getitem__(self, index): with self.assertRaises(TypeError): max(1, 2, default=0) + # Test iterables that can only be looped once #510 + self.assertEqual(min(x for x in [5]), 5) + self.assertEqual(max(x for x in [5, 4, 3]), 5) + def test_next(self): it = iter(range(2)) self.assertEqual(next(it), 0) From 03f94a325251f4ef19d4b4caabc72a5a88c60172 Mon Sep 17 00:00:00 2001 From: Chase Sterling Date: Thu, 17 Oct 2019 21:19:25 -0400 Subject: [PATCH 2/5] Fix behavior of new min/max with generators. fix #510 --- src/future/builtins/new_min_max.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/future/builtins/new_min_max.py b/src/future/builtins/new_min_max.py index 8fd63fdf..cc7acbee 100644 --- a/src/future/builtins/new_min_max.py +++ b/src/future/builtins/new_min_max.py @@ -1,3 +1,5 @@ +import itertools + from future import utils if utils.PY2: from __builtin__ import max as _builtin_max, min as _builtin_min @@ -33,17 +35,20 @@ def new_min_max(_builtin_func, *args, **kwargs): raise TypeError if len(args) == 1: + iterator = iter(args[0]) try: - next(iter(args[0])) + first = next(iterator) except StopIteration: if kwargs.get('default') is not None: return kwargs.get('default') else: raise ValueError('iterable is an empty sequence') + else: + iterator = itertools.chain([first], iterator) if kwargs.get('key') is not None: - return _builtin_func(args[0], key=kwargs.get('key')) + return _builtin_func(iterator, key=kwargs.get('key')) else: - return _builtin_func(args[0]) + return _builtin_func(iterator) if len(args) > 1: if kwargs.get('key') is not None: From b64245c8444d93aee6862bb0cd2361ddfe431eea Mon Sep 17 00:00:00 2001 From: Chase Sterling Date: Fri, 18 Oct 2019 09:24:57 -0400 Subject: [PATCH 3/5] Move max test to appropriate test method --- tests/test_future/test_builtins.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_future/test_builtins.py b/tests/test_future/test_builtins.py index 29f4c84a..d5ea8972 100644 --- a/tests/test_future/test_builtins.py +++ b/tests/test_future/test_builtins.py @@ -1123,6 +1123,7 @@ class BadSeq: def __getitem__(self, index): raise ValueError self.assertRaises(ValueError, min, BadSeq()) + self.assertEqual(max(x for x in [5, 4, 3]), 5) for stmt in ( "min(key=int)", # no args @@ -1156,7 +1157,6 @@ def __getitem__(self, index): # Test iterables that can only be looped once #510 self.assertEqual(min(x for x in [5]), 5) - self.assertEqual(max(x for x in [5, 4, 3]), 5) def test_next(self): it = iter(range(2)) From dbc74ad81b384897970edd65fef1bc53ddee5acb Mon Sep 17 00:00:00 2001 From: Chase Sterling Date: Fri, 18 Oct 2019 10:31:21 -0400 Subject: [PATCH 4/5] Fix min/max functions not allowing 'None' as default --- src/future/builtins/new_min_max.py | 6 ++++-- tests/test_future/test_builtins.py | 2 ++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/future/builtins/new_min_max.py b/src/future/builtins/new_min_max.py index cc7acbee..1bfd8a90 100644 --- a/src/future/builtins/new_min_max.py +++ b/src/future/builtins/new_min_max.py @@ -6,6 +6,8 @@ else: from builtins import max as _builtin_max, min as _builtin_min +_SENTINEL = object() + def newmin(*args, **kwargs): return new_min_max(_builtin_min, *args, **kwargs) @@ -31,7 +33,7 @@ def new_min_max(_builtin_func, *args, **kwargs): if len(args) == 0: raise TypeError - if len(args) != 1 and kwargs.get('default') is not None: + if len(args) != 1 and kwargs.get('default', _SENTINEL) is not _SENTINEL: raise TypeError if len(args) == 1: @@ -39,7 +41,7 @@ def new_min_max(_builtin_func, *args, **kwargs): try: first = next(iterator) except StopIteration: - if kwargs.get('default') is not None: + if kwargs.get('default', _SENTINEL) is not _SENTINEL: return kwargs.get('default') else: raise ValueError('iterable is an empty sequence') diff --git a/tests/test_future/test_builtins.py b/tests/test_future/test_builtins.py index d5ea8972..ca07b9ef 100644 --- a/tests/test_future/test_builtins.py +++ b/tests/test_future/test_builtins.py @@ -1105,6 +1105,7 @@ def test_max(self): with self.assertRaises(TypeError): max(1, 2, default=0) self.assertEqual(max([], default=0), 0) + self.assertIs(max([], default=None), None) def test_min(self): self.assertEqual(min('123123'), '1') @@ -1150,6 +1151,7 @@ def __getitem__(self, index): sorted(data, key=f)[0]) self.assertEqual(min([], default=5), 5) self.assertEqual(min([], default=0), 0) + self.assertIs(min([], default=None), None) with self.assertRaises(TypeError): max(None, default=5) with self.assertRaises(TypeError): From f4926e51c940ef9afb4c94038c48b8a5382d04a2 Mon Sep 17 00:00:00 2001 From: Chase Sterling Date: Fri, 18 Oct 2019 10:32:22 -0400 Subject: [PATCH 5/5] Make new min/max error message consistent builtin error message --- src/future/builtins/new_min_max.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/future/builtins/new_min_max.py b/src/future/builtins/new_min_max.py index 1bfd8a90..6f0c2a86 100644 --- a/src/future/builtins/new_min_max.py +++ b/src/future/builtins/new_min_max.py @@ -44,7 +44,7 @@ def new_min_max(_builtin_func, *args, **kwargs): if kwargs.get('default', _SENTINEL) is not _SENTINEL: return kwargs.get('default') else: - raise ValueError('iterable is an empty sequence') + raise ValueError('{}() arg is an empty sequence'.format(_builtin_func.__name__)) else: iterator = itertools.chain([first], iterator) if kwargs.get('key') is not None: