From f7692af93190af988909bec31fe26f8792aa70f3 Mon Sep 17 00:00:00 2001 From: Andrey Darascheka Date: Mon, 24 Feb 2020 10:59:02 +0300 Subject: [PATCH 1/6] bpo-39728: enum with invalid value results in ValueError twice --- Lib/enum.py | 1 - Lib/test/test_enum.py | 15 ++++----------- .../2020-02-24-10-58-34.bpo-39728.kOOaHn.rst | 1 + 3 files changed, 5 insertions(+), 12 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2020-02-24-10-58-34.bpo-39728.kOOaHn.rst diff --git a/Lib/enum.py b/Lib/enum.py index e72d3062674618..fe234d5de26b0d 100644 --- a/Lib/enum.py +++ b/Lib/enum.py @@ -606,7 +606,6 @@ def __new__(cls, value): 'error in %s._missing_: returned %r instead of None or a valid member' % (cls.__name__, result) ) - exc.__context__ = ve_exc raise exc def _generate_next_value_(name, start, count, last_values): diff --git a/Lib/test/test_enum.py b/Lib/test/test_enum.py index 865edf1d9cfc67..c4c34c66a2389b 100644 --- a/Lib/test/test_enum.py +++ b/Lib/test/test_enum.py @@ -1860,19 +1860,12 @@ def _missing_(cls, item): # trigger not found return None self.assertIs(Color('three'), Color.blue) - self.assertRaises(ValueError, Color, 7) - try: + with self.assertRaises(ValueError): + Color(7) + with self.assertRaises(TypeError): Color('bad return') - except TypeError as exc: - self.assertTrue(isinstance(exc.__context__, ValueError)) - else: - raise Exception('Exception not raised.') - try: + with self.assertRaises(ZeroDivisionError): Color('error out') - except ZeroDivisionError as exc: - self.assertTrue(isinstance(exc.__context__, ValueError)) - else: - raise Exception('Exception not raised.') def test_multiple_mixin(self): class MaxMixin: diff --git a/Misc/NEWS.d/next/Library/2020-02-24-10-58-34.bpo-39728.kOOaHn.rst b/Misc/NEWS.d/next/Library/2020-02-24-10-58-34.bpo-39728.kOOaHn.rst new file mode 100644 index 00000000000000..d9599f0ceb5e6a --- /dev/null +++ b/Misc/NEWS.d/next/Library/2020-02-24-10-58-34.bpo-39728.kOOaHn.rst @@ -0,0 +1 @@ +Instantiating enum with invalid value results in ValueError twice From 88a2186da9df3309391d566f3deb90204c309c4f Mon Sep 17 00:00:00 2001 From: Andrey Darascheka Date: Mon, 24 Feb 2020 12:50:55 +0300 Subject: [PATCH 2/6] bpo-39728: Fixed after review --- Lib/enum.py | 1 + Lib/test/test_enum.py | 12 ++++++++++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/Lib/enum.py b/Lib/enum.py index fe234d5de26b0d..bbe6b13c7add8d 100644 --- a/Lib/enum.py +++ b/Lib/enum.py @@ -606,6 +606,7 @@ def __new__(cls, value): 'error in %s._missing_: returned %r instead of None or a valid member' % (cls.__name__, result) ) + exc.__context__ = ve_exc raise exc def _generate_next_value_(name, start, count, last_values): diff --git a/Lib/test/test_enum.py b/Lib/test/test_enum.py index c4c34c66a2389b..4a1c04dfc1fcf5 100644 --- a/Lib/test/test_enum.py +++ b/Lib/test/test_enum.py @@ -1862,10 +1862,18 @@ def _missing_(cls, item): self.assertIs(Color('three'), Color.blue) with self.assertRaises(ValueError): Color(7) - with self.assertRaises(TypeError): + try: Color('bad return') - with self.assertRaises(ZeroDivisionError): + except TypeError as exc: + self.assertTrue(isinstance(exc.__context__, ValueError)) + else: + raise Exception('Exception not raised.') + try: Color('error out') + except ZeroDivisionError as exc: + self.assertTrue(not exc.__context__) + else: + raise Exception('Exception not raised.') def test_multiple_mixin(self): class MaxMixin: From a442c526c2688e19d868776736af36a2372ee3f8 Mon Sep 17 00:00:00 2001 From: Ethan Furman Date: Wed, 16 Sep 2020 06:24:06 -0700 Subject: [PATCH 3/6] always attach ValueError context when `_missing_` is called a `ValueError` has occurred, so any exception raised by `_missing_` must have it's context set to that `ValueError` --- Lib/enum.py | 2 +- Lib/test/test_enum.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/enum.py b/Lib/enum.py index bbe6b13c7add8d..e72d3062674618 100644 --- a/Lib/enum.py +++ b/Lib/enum.py @@ -606,7 +606,7 @@ def __new__(cls, value): 'error in %s._missing_: returned %r instead of None or a valid member' % (cls.__name__, result) ) - exc.__context__ = ve_exc + exc.__context__ = ve_exc raise exc def _generate_next_value_(name, start, count, last_values): diff --git a/Lib/test/test_enum.py b/Lib/test/test_enum.py index 4a1c04dfc1fcf5..6c767af7b747a6 100644 --- a/Lib/test/test_enum.py +++ b/Lib/test/test_enum.py @@ -1871,7 +1871,7 @@ def _missing_(cls, item): try: Color('error out') except ZeroDivisionError as exc: - self.assertTrue(not exc.__context__) + self.assertTrue(isinstance(exc.__context__, ValueError)) else: raise Exception('Exception not raised.') From 6965a6c9ece6b5f8cff213a9b6b979c712c97102 Mon Sep 17 00:00:00 2001 From: Ethan Furman Date: Wed, 16 Sep 2020 06:28:55 -0700 Subject: [PATCH 4/6] fix default `_missing_` `_missing_` should return `None` if it cannot match the value; it should not raise a `ValueError` itself -- the method that calls `_missing_` will take care of that --- Lib/enum.py | 2 +- Lib/test/test_enum.py | 18 +++++++++++++++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/Lib/enum.py b/Lib/enum.py index e72d3062674618..35a045f9ab1824 100644 --- a/Lib/enum.py +++ b/Lib/enum.py @@ -620,7 +620,7 @@ def _generate_next_value_(name, start, count, last_values): @classmethod def _missing_(cls, value): - raise ValueError("%r is not a valid %s" % (value, cls.__qualname__)) + return None def __repr__(self): return "<%s.%s: %r>" % ( diff --git a/Lib/test/test_enum.py b/Lib/test/test_enum.py index 6c767af7b747a6..697a4aee4f29f1 100644 --- a/Lib/test/test_enum.py +++ b/Lib/test/test_enum.py @@ -1842,6 +1842,18 @@ class Dupes(Enum): third = auto() self.assertEqual([Dupes.first, Dupes.second, Dupes.third], list(Dupes)) + def test_default_missing(self): + class Color(Enum): + RED = 1 + GREEN = 2 + BLUE = 3 + try: + Color(7) + except ValueError as exc: + self.assertTrue(exc.__context__ is None) + else: + raise Exception('Exception not raised.') + def test_missing(self): class Color(Enum): red = 1 @@ -1860,8 +1872,12 @@ def _missing_(cls, item): # trigger not found return None self.assertIs(Color('three'), Color.blue) - with self.assertRaises(ValueError): + try: Color(7) + except ValueError as exc: + self.assertTrue(exc.__context__ is None) + else: + raise Exception('Exception not raised.') try: Color('bad return') except TypeError as exc: From 6d45aa362738ead75a4c36b0baae039f03de74a6 Mon Sep 17 00:00:00 2001 From: Ethan Furman Date: Wed, 16 Sep 2020 06:43:41 -0700 Subject: [PATCH 5/6] update NEWS entry --- .../next/Library/2020-02-24-10-58-34.bpo-39728.kOOaHn.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Library/2020-02-24-10-58-34.bpo-39728.kOOaHn.rst b/Misc/NEWS.d/next/Library/2020-02-24-10-58-34.bpo-39728.kOOaHn.rst index d9599f0ceb5e6a..beb2016a85ba68 100644 --- a/Misc/NEWS.d/next/Library/2020-02-24-10-58-34.bpo-39728.kOOaHn.rst +++ b/Misc/NEWS.d/next/Library/2020-02-24-10-58-34.bpo-39728.kOOaHn.rst @@ -1 +1 @@ -Instantiating enum with invalid value results in ValueError twice +fix default `_missing_` so a duplicate `ValueError` is not set as the `__context__` of the original `ValueError` From bb843464ce92e5322552300ddf2cbc7dd7f7b59b Mon Sep 17 00:00:00 2001 From: Ethan Furman Date: Wed, 16 Sep 2020 07:08:37 -0700 Subject: [PATCH 6/6] add Andrey Doroschenko to ACKS --- Misc/ACKS | 1 + 1 file changed, 1 insertion(+) diff --git a/Misc/ACKS b/Misc/ACKS index 8b0d7a45da1695..2628d6b690d1cb 100644 --- a/Misc/ACKS +++ b/Misc/ACKS @@ -433,6 +433,7 @@ Marcos Donolo Dima Dorfman Yves Dorfsman Michael Dorman +Andrey Doroschenko Steve Dower Allen Downey Cesar Douady