diff --git a/src/core/IronPython/Modules/Builtin.cs b/src/core/IronPython/Modules/Builtin.cs index 4189fd56a..833119b73 100644 --- a/src/core/IronPython/Modules/Builtin.cs +++ b/src/core/IronPython/Modules/Builtin.cs @@ -792,10 +792,12 @@ public static object locals(CodeContext/*!*/ context) { private static readonly object UndefinedKeywordArgument = new object(); - public static object? max(CodeContext/*!*/ context, object? x) { + private static object? MaxWithDefault(CodeContext/*!*/ context, object? x, object? def) { IEnumerator i = PythonOps.GetEnumerator(context, x); - if (!i.MoveNext()) + if (!i.MoveNext()) { + if (def != UndefinedKeywordArgument) return def; throw PythonOps.ValueError("max() arg is an empty sequence"); + } object? ret = i.Current; PythonContext pc = context.LanguageContext; while (i.MoveNext()) { @@ -804,9 +806,11 @@ public static object locals(CodeContext/*!*/ context) { return ret; } - public static object? max(CodeContext/*!*/ context, object? x, object? y) { - return context.LanguageContext.GreaterThan(x, y) ? x : y; - } + public static object? max(CodeContext/*!*/ context, object? x) + => MaxWithDefault(context, x, UndefinedKeywordArgument); + + public static object? max(CodeContext/*!*/ context, object? x, object? y) + => context.LanguageContext.GreaterThan(y, x) ? y : x; public static object? max(CodeContext/*!*/ context, [NotNone] params object?[] args) { if (args.Length > 0) { @@ -829,21 +833,18 @@ public static object locals(CodeContext/*!*/ context) { } public static object? max(CodeContext/*!*/ context, object? x, [ParamDictionary] IDictionary dict) { - IEnumerator i = PythonOps.GetEnumerator(context, x); + VerifyKeys(nameof(max), dict, out object? method, out object? def); - var kwargTuple = GetMaxKwArg(dict, isDefaultAllowed: true); - object? method = kwargTuple.Item1; - object? def = kwargTuple.Item2; + if (method == UndefinedKeywordArgument || method == null) { // null key is allowed since CPython 3.8 + return MaxWithDefault(context, x, def); + } + IEnumerator i = PythonOps.GetEnumerator(context, x); if (!i.MoveNext()) { if (def != UndefinedKeywordArgument) return def; throw PythonOps.ValueError("max() arg is an empty sequence"); } - if (method == UndefinedKeywordArgument || method == null) { // null key is allowed since CPython 3.8 - return max(context, x); - } - object? ret = i.Current; object? retValue = PythonCalls.Call(context, method, i.Current); PythonContext pc = context.LanguageContext; @@ -858,57 +859,40 @@ public static object locals(CodeContext/*!*/ context) { } public static object? max(CodeContext/*!*/ context, object? x, object? y, [ParamDictionary] IDictionary dict) { - var kwargTuple = GetMaxKwArg(dict, isDefaultAllowed: false); - object? method = kwargTuple.Item1; + VerifyKeysNoDefault(nameof(max), dict, out object? method); if (method == null) return max(context, x, y); // null key is allowed since CPython 3.8 PythonContext pc = context.LanguageContext; - return pc.GreaterThan(PythonCalls.Call(context, method, x), PythonCalls.Call(context, method, y)) ? x : y; + return pc.GreaterThan(PythonCalls.Call(context, method, y), PythonCalls.Call(context, method, x)) ? y : x; } public static object? max(CodeContext/*!*/ context, [ParamDictionary] IDictionary dict, [NotNone] params object?[] args) { - var kwargTuple = GetMaxKwArg(dict, isDefaultAllowed: false); - object? method = kwargTuple.Item1; + if (args.Length == 0) throw PythonOps.TypeError("max expected 1 arguments, got 0"); - if (method == null) return max(context, args); // null key is allowed since CPython 3.8 + VerifyKeysNoDefault(nameof(max), dict, out object? method); - if (args.Length > 0) { - int retIndex = 0; - if (args.Length == 1) { - return max(context, args[retIndex], dict); - } - - object? retValue = PythonCalls.Call(context, method, args[retIndex]); - PythonContext pc = context.LanguageContext; - for (int i = 1; i < args.Length; i++) { - object? tmpRetValue = PythonCalls.Call(context, method, args[i]); + if (method == null) return max(context, args); // null key is allowed since CPython 3.8 + if (args.Length == 1) return max(context, args[0], dict); - if (pc.GreaterThan(tmpRetValue, retValue)) { - retIndex = i; - retValue = tmpRetValue; - } + int retIndex = 0; + object? retValue = PythonCalls.Call(context, method, args[retIndex]); + PythonContext pc = context.LanguageContext; + for (int i = 1; i < args.Length; i++) { + object? tmpRetValue = PythonCalls.Call(context, method, args[i]); + if (pc.GreaterThan(tmpRetValue, retValue)) { + retIndex = i; + retValue = tmpRetValue; } - return args[retIndex]; - } else { - throw PythonOps.TypeError("max expected 1 arguments, got 0"); } + return args[retIndex]; } - private static Tuple GetMaxKwArg(IDictionary dict, bool isDefaultAllowed) { - if (dict.Count != 1 && dict.Count != 2) - throw PythonOps.TypeError("max() should have only 2 keyword arguments, but got {0} keyword arguments", dict.Count); - if (dict.ContainsKey("default") && !isDefaultAllowed) { - throw PythonOps.TypeError("Cannot specify a default for max() with multiple positional arguments"); - } - - return VerifyKeys("max", dict); - } - - public static object? min(CodeContext/*!*/ context, object? x) { + private static object? MinWithDefault(CodeContext/*!*/ context, object? x, object? def) { IEnumerator i = PythonOps.GetEnumerator(context, x); if (!i.MoveNext()) { - throw PythonOps.ValueError("empty sequence"); + if (def != UndefinedKeywordArgument) return def; + throw PythonOps.ValueError("min() arg is an empty sequence"); } object? ret = i.Current; PythonContext pc = context.LanguageContext; @@ -918,9 +902,11 @@ public static object locals(CodeContext/*!*/ context) { return ret; } - public static object? min(CodeContext/*!*/ context, object? x, object? y) { - return context.LanguageContext.LessThan(x, y) ? x : y; - } + public static object? min(CodeContext/*!*/ context, object? x) + => MinWithDefault(context, x, UndefinedKeywordArgument); + + public static object? min(CodeContext/*!*/ context, object? x, object? y) + => context.LanguageContext.LessThan(y, x) ? y : x; public static object? min(CodeContext/*!*/ context, [NotNone] params object?[] args) { if (args.Length > 0) { @@ -940,19 +926,18 @@ public static object locals(CodeContext/*!*/ context) { } public static object? min(CodeContext/*!*/ context, object? x, [ParamDictionary] IDictionary dict) { + VerifyKeys(nameof(min), dict, out object? method, out object? def); + + if (method == UndefinedKeywordArgument || method == null) { // null key is allowed since CPython 3.8 + return MinWithDefault(context, x, def); + } + IEnumerator i = PythonOps.GetEnumerator(context, x); - var kwargTuple = GetMinKwArg(dict, isDefaultAllowed: true); - object? method = kwargTuple.Item1; - object? def = kwargTuple.Item2; if (!i.MoveNext()) { if (def != UndefinedKeywordArgument) return def; throw PythonOps.ValueError("min() arg is an empty sequence"); } - if (method == UndefinedKeywordArgument || method == null) { // null key is allowed since CPython 3.8 - return min(context, x); - } - object? ret = i.Current; object? retValue = PythonCalls.Call(context, method, i.Current); PythonContext pc = context.LanguageContext; @@ -967,82 +952,59 @@ public static object locals(CodeContext/*!*/ context) { } public static object? min(CodeContext/*!*/ context, object? x, object? y, [ParamDictionary] IDictionary dict) { - var kwargTuple = GetMinKwArg(dict, isDefaultAllowed: false); - object? method = kwargTuple.Item1; + VerifyKeysNoDefault(nameof(min), dict, out object? method); if (method == null) return min(context, x, y); // null key is allowed since CPython 3.8 - return context.LanguageContext.LessThan(PythonCalls.Call(context, method, x), PythonCalls.Call(context, method, y)) ? x : y; + return context.LanguageContext.LessThan(PythonCalls.Call(context, method, y), PythonCalls.Call(context, method, x)) ? y : x; } public static object? min(CodeContext/*!*/ context, [ParamDictionary] IDictionary dict, [NotNone] params object?[] args) { - var kwargTuple = GetMinKwArg(dict, isDefaultAllowed: false); - object? method = kwargTuple.Item1; + if (args.Length == 0) throw PythonOps.TypeError("min expected 1 arguments, got 0"); - if (method == null) return min(context, args); // null key is allowed since CPython 3.8 + VerifyKeysNoDefault(nameof(min), dict, out object? method); - if (args.Length > 0) { - int retIndex = 0; - if (args.Length == 1) { - return min(context, args[retIndex], dict); - } - - object? retValue = PythonCalls.Call(context, method, args[retIndex]); - PythonContext pc = context.LanguageContext; - - for (int i = 1; i < args.Length; i++) { - object? tmpRetValue = PythonCalls.Call(context, method, args[i]); + if (method == null) return min(context, args); // null key is allowed since CPython 3.8 + if (args.Length == 1) return min(context, args[0], dict); - if (pc.LessThan(tmpRetValue, retValue)) { - retIndex = i; - retValue = tmpRetValue; - } + int retIndex = 0; + object? retValue = PythonCalls.Call(context, method, args[retIndex]); + PythonContext pc = context.LanguageContext; + for (int i = 1; i < args.Length; i++) { + object? tmpRetValue = PythonCalls.Call(context, method, args[i]); + if (pc.LessThan(tmpRetValue, retValue)) { + retIndex = i; + retValue = tmpRetValue; } - return args[retIndex]; - } else { - throw PythonOps.TypeError("min expected 1 arguments, got 0"); } + return args[retIndex]; } - private static Tuple GetMinKwArg(IDictionary dict, bool isDefaultAllowed) { - if (dict.Count != 1 && dict.Count != 2) - throw PythonOps.TypeError("min() should have only 2 keyword arguments, but got {0} keyword arguments", dict.Count); - - if (dict.ContainsKey("default") && !isDefaultAllowed) - throw PythonOps.TypeError("Cannot specify a default for min() with multiple positional arguments"); - - return VerifyKeys("min", dict); + private static void VerifyKeysNoDefault(string name, IDictionary dict, out object? method) { + VerifyKeys(name, dict, out method, out object? def); + if (def != UndefinedKeywordArgument) + throw PythonOps.TypeError("Cannot specify a default for {0}() with multiple positional arguments", name); } - private static Tuple VerifyKeys(string name, IDictionary dict) { - object? method; - object? def; - - int cnt = 0; - if (dict.TryGetValue("key", out method)) { - cnt++; - } else { - method = UndefinedKeywordArgument; - } - - if (dict.TryGetValue("default", out def)) { - cnt++; - } else { - def = UndefinedKeywordArgument; - } - + private static void VerifyKeys(string name, IDictionary dict, out object? method, out object? def) { if (dict.Count > 2) { - throw PythonOps.TypeError("{1}() takes at most 2 keyword arguments ({0} given)", dict.Count, name); + throw PythonOps.TypeError("{0}() takes at most 2 keyword arguments ({1} given)", name, dict.Count); } - if (dict.Count > cnt) { - var key = dict.Keys.Cast().Where(x => x != "default" && x != "key").First(); - throw PythonOps.TypeError("{1}() got an unexpected keyword argument ({0})", key, name); - } - - var result = Tuple.Create(method, def); + method = def = UndefinedKeywordArgument; - return result; + foreach (var pair in dict) { + switch (pair.Key) { + case "key": + method = pair.Value; + break; + case "default": + def = pair.Value; + break; + default: + throw PythonOps.TypeError("{1}() got an unexpected keyword argument '{0}'", pair.Key, name); + } + } } /// diff --git a/tests/suite/test_builtinfunc.py b/tests/suite/test_builtinfunc.py index 854e3b235..ba179ffb9 100644 --- a/tests/suite/test_builtinfunc.py +++ b/tests/suite/test_builtinfunc.py @@ -318,9 +318,14 @@ def test_max(self): self.assertEqual(max(1, 2.0, 3), 3) self.assertEqual(max(1.0, 2, 3), 3) - self.assertRaises(TypeError, max) + self.assertRaisesRegex(TypeError, "expected", max) + self.assertRaisesRegex(TypeError, "expected", max, key=int) + self.assertRaisesRegex(TypeError, "expected", max, default=None) + self.assertRaisesRegex(TypeError, "expected", max, default=None, key=int) self.assertRaises(TypeError, max, 42) - self.assertRaises(ValueError, max, ()) + self.assertRaisesRegex(ValueError, "empty", max, ()) + self.assertRaisesRegex(ValueError, "empty", max, iter(())) + self.assertRaisesRegex(ValueError, "empty", max, (), key=neg) self.assertRaises(TypeError, max, (1, 2), None) class BadSeq: def __getitem__(self, index): @@ -358,6 +363,20 @@ def __getitem__(self, index): f = keys.__getitem__ self.assertEqual(max(data, key=f), sorted(reversed(data), key=f)[-1]) + # iterator with default and no key + self.assertEqual(max(iter([2, 1, 0]), default=None), 2) + self.assertEqual(max(iter([]), default=None), None) + self.assertEqual(max([], key=None, default=1), 1) + + # first max item wins + self.assertIs(type(max(1, 1.0)), int) + self.assertIs(type(max(1, 1.0, key=neg)), int) + self.assertIs(type(max([1, 1.0])), int) + self.assertIs(type(max(0, 1, 1.0)), int) + self.assertIs(type(max(0, 1, 1.0, key=abs)), int) + self.assertIs(type(max([0, 1, 1.0], key=abs)), int) + self.assertTrue(math.isnan(max(float('nan'), 1))) + self.assertEqual(max(1, float('nan')), 1) def test_min(self): self.assertEqual(min('123123'), '1') @@ -369,9 +388,14 @@ def test_min(self): self.assertEqual(min(1, 2.0, 3), 1) self.assertEqual(min(1.0, 2, 3), 1.0) - self.assertRaises(TypeError, min) + self.assertRaisesRegex(TypeError, "expected", min) + self.assertRaisesRegex(TypeError, "expected", min, key=int) + self.assertRaisesRegex(TypeError, "expected", min, default=None) + self.assertRaisesRegex(TypeError, "expected", min, default=None, key=int) self.assertRaises(TypeError, min, 42) - self.assertRaises(ValueError, min, ()) + self.assertRaisesRegex(ValueError, "empty", min, ()) + self.assertRaisesRegex(ValueError, "empty", min, iter(())) + self.assertRaisesRegex(ValueError, "empty", min, (), key=neg) class BadSeq: def __getitem__(self, index): raise ValueError @@ -410,6 +434,21 @@ def __getitem__(self, index): f = keys.__getitem__ self.assertEqual(min(data, key=f), sorted(data, key=f)[0]) + # iterator with default and no key + self.assertEqual(min(iter([0, 1, 2]), default=None), 0) + self.assertEqual(min(iter([]), default=None), None) + self.assertEqual(min([], key=None, default=1), 1) + + # first min item wins + self.assertIs(type(min(1, 1.0)), int) + self.assertIs(type(min(1, 1.0, key=neg)), int) + self.assertIs(type(min([1, 1.0])), int) + self.assertIs(type(min(2, 1, 1.0)), int) + self.assertIs(type(min(2, 1, 1.0, key=abs)), int) + self.assertIs(type(min([2, 1, 1.0], key=abs)), int) + self.assertTrue(math.isnan(min(float('nan'), 1))) + self.assertEqual(min(1, float('nan')), 1) + def test_abs(self): self.assertRaises(TypeError,abs,None)