gh-116040: [Enum] fix by-value calls when second value is falsey; e.g. Cardinal(1, 0) by ethanfurman · Pull Request #116072 · python/cpython

ethanfurman · 2024-02-28T21:13:47Z

Issue: Enum creation from values fails for certain values. #116040

serhiy-storchaka · 2024-02-29T14:08:50Z

Lib/test/test_enum.py

+        Cardinal(1, 0)
+        Cardinal(-1, 0)


Could you test that the result is correct?

serhiy-storchaka · 2024-02-29T14:23:03Z

Lib/enum.py

        return cls._create_(
                class_name=value,
-                names=names,
+                names=names or None,


Add tests for something like:

Enum('empty_enum', '')

Enum('empty_enum', [])

Enum('empty_enum', {})

Enum('empty_enum', '', type=int)

Enum('empty_enum', [], type=int)

Enum('empty_enum', {}, type=int)

serhiy-storchaka · 2024-02-29T14:23:13Z

Lib/enum.py

+    def __bool__(self):
+        return False


Is it needed?

Yes, or the or None at line 730 will fail.

Perhaps it should not take the boolean value there at first place, but use is not _not_given? I suspected that it can produce weird results if the second value is false and type is given.

I'm not sure what you mean -- is "second value" the names parameter? If it's falsey and a type is given, a new empty enum is created (which is appropriate).

miss-islington-app · 2024-03-07T21:30:29Z

Thanks @ethanfurman for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

miss-islington-app · 2024-03-07T21:30:40Z

Sorry, @ethanfurman, I could not cleanly backport this to 3.12 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 13ffd4bd9f529b6a5fe33741fbd57f14b4b80137 3.12

…y; e.g. Cardinal(1, 0) (pythonGH-116072) (cherry picked from commit 13ffd4b)

bedevere-app · 2024-03-07T21:54:40Z

GH-116476 is a backport of this pull request to the 3.12 branch.

…ey (GH-116072) (GH-116476) e.g. Cardinal(1, 0) (cherry picked from commit 13ffd4b)

serhiy-storchaka

Sorry for not commenting at the time, but something looks suspicious in this change.

serhiy-storchaka · 2024-03-08T07:55:46Z

Lib/enum.py

+    def __bool__(self):
+        return False


Perhaps it should not take the boolean value there at first place, but use is not _not_given? I suspected that it can produce weird results if the second value is false and type is given.

serhiy-storchaka · 2024-03-08T07:57:02Z

Lib/test/test_enum.py

+            ):
+            empty_enum = Enum('empty_enum', nothing, type=e_type)
+            self.assertEqual(len(empty_enum), 0)
+            self.assertRaises(TypeError, 'has no members', empty_enum, 0)


It raises a TypeError because you try to call 'has no members'(empty_enum, 0).

Gah. Changed to assertRaisesRegex.

serhiy-storchaka · 2024-03-08T07:59:56Z

Lib/test/test_enum.py

+        for nothing, e_type in (
+                ('', None),
+                ('', int),
+                ([], None),
+                ([], int),
+                ({}, None),
+                ({}, int),
+            ):


It could perhaps be clearer with nested loops.

for nothing in '', [], {}: for e_type in None, int: ...

Good point.

ethanfurman · 2024-03-08T18:33:04Z

@serhiy-storchaka, fixes in #116508.

…y; e.g. Cardinal(1, 0) (pythonGH-116072)

fix by-value calls when second value is falsey; e.g. Cardinal(1, 0)

a5828a6

ethanfurman self-assigned this Feb 28, 2024

bedevere-app bot added the awaiting core review label Feb 28, 2024

bedevere-app bot mentioned this pull request Feb 28, 2024

Enum creation from values fails for certain values. #116040

Closed

ethanfurman removed 3.11 only security fixes needs backport to 3.11 only security fixes labels Feb 28, 2024

Merge branch 'main' into enum-zero_not_none

f04a765

serhiy-storchaka reviewed Feb 29, 2024

View reviewed changes

address review comments

248bd2c

ethanfurman requested a review from serhiy-storchaka March 4, 2024 20:41

ethanfurman merged commit 13ffd4b into python:main Mar 7, 2024

bedevere-app bot removed the awaiting core review label Mar 7, 2024

ethanfurman added a commit to ethanfurman/cpython that referenced this pull request Mar 7, 2024

pythongh-116040: [Enum] fix by-value calls when second value is false…

82802f6

…y; e.g. Cardinal(1, 0) (pythonGH-116072) (cherry picked from commit 13ffd4b)

bedevere-app bot removed the needs backport to 3.12 only security fixes label Mar 7, 2024

ethanfurman added a commit that referenced this pull request Mar 7, 2024

[3.12] gh-116040: [Enum] fix by-value calls when second value is fals…

d771729

…ey (GH-116072) (GH-116476) e.g. Cardinal(1, 0) (cherry picked from commit 13ffd4b)

ethanfurman deleted the enum-zero_not_none branch March 8, 2024 00:00

serhiy-storchaka reviewed Mar 8, 2024

View reviewed changes

adorilson pushed a commit to adorilson/cpython that referenced this pull request Mar 25, 2024

pythongh-116040: [Enum] fix by-value calls when second value is false…

a7a60b8

…y; e.g. Cardinal(1, 0) (pythonGH-116072)

diegorusso pushed a commit to diegorusso/cpython that referenced this pull request Apr 17, 2024

pythongh-116040: [Enum] fix by-value calls when second value is false…

5e9b76e

…y; e.g. Cardinal(1, 0) (pythonGH-116072)

Uh oh!

Conversation

ethanfurman commented Feb 28, 2024 • edited by bedevere-app bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

miss-islington-app bot commented Mar 7, 2024

Uh oh!

miss-islington-app bot commented Mar 7, 2024

Uh oh!

bedevere-app bot commented Mar 7, 2024

Uh oh!

serhiy-storchaka left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ethanfurman commented Mar 8, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ethanfurman commented Feb 28, 2024 •

edited by bedevere-app bot

Loading