bpo-40084: Enum.dir listing includes entries from instance dict #19219

lem2clide · 2020-03-29T20:56:07Z

https://bugs.python.org/issue40084

the-knights-who-say-ni · 2020-03-29T20:56:10Z

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA).

CLA Missing

Our records indicate the following people have not signed the CLA:

@lem2clide

For legal reasons we need all the people listed to sign the CLA before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

If you have recently signed the CLA, please wait at least one business day
before our records are updated.

You can check yourself to see if the CLA has been received.

Thanks again for the contribution, we look forward to reviewing it!

rhettinger

Please add tests including the for http example. Also add a news/entry.

bedevere-bot · 2020-03-30T00:07:39Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

dimaqq · 2020-03-30T03:09:53Z

Lib/enum.py

@@ -614,7 +614,7 @@ def __dir__(self):
                for cls in self.__class__.mro()
                for m in cls.__dict__
                if m[0] != '_' and m not in self._member_map_
-                ]
+                ] + [m for m in self.__dict__ if m[0] != '_']


Personally I'd prefer if not m.startswith("_") because it's more Pythonic and resilient in case someone smuggles an empty string in.
But, perhaps, m[0] is more performant?

Performance isn't really an issue for a function like dir() (except in extreme cases, of course). In this case we'll stick with m[0] to match what's already being used.

It seems that m[0] is more performant and I also wanted to keep the consistency with the previous comparison m[0] != '_'

I don't know why it more performant though, I've found the following infos about what is performed:

m[0] uses COMPARE_OP which seems to use opcode prediction, paired with POP_JUMP_IF_FALSE and POP_JUMP_IF_TRUE

startswith implies a call of the builtin function

Thanks for asking, I've learned more on the opcode dispatch while investigating!

ethanfurman

Looking good! Change the News entry, add yourself to Misc/ACKS if you're not already there, and we should be good to go.

ethanfurman · 2020-03-30T20:26:22Z

Misc/NEWS.d/next/Library/2020-03-29-21-32-00.bpo-40084.MCYwcv.rst

@@ -0,0 +1 @@
+Enum.__dir__ listing includes entries from instance dict


dir(Enum.member) now includes attributes as well as methods.

ethanfurman · 2020-03-30T20:28:07Z

Lib/enum.py

@@ -614,7 +614,7 @@ def __dir__(self):
                for cls in self.__class__.mro()
                for m in cls.__dict__
                if m[0] != '_' and m not in self._member_map_
-                ]
+                ] + [m for m in self.__dict__ if m[0] != '_']


Performance isn't really an issue for a function like dir() (except in extreme cases, of course). In this case we'll stick with m[0] to match what's already being used.

lem2clide · 2020-03-30T20:59:56Z

Looking good! Change the News entry, add yourself to Misc/ACKS if you're not already there, and we should be good to go.

I have made the requested changes; please review again.

fixed News entry
added myself to Misc/ACKS

thanks for your review :)

rhettinger · 2020-03-31T22:47:41Z

Lib/test/test_enum.py

@@ -173,6 +173,9 @@ class Holiday(date, Enum):
            IDES_OF_MARCH = 2013, 3, 15
        self.Holiday = Holiday

+        from http import HTTPStatus
+        self.HTTPStatus = HTTPStatus


This test should be in the test for the http module. Also, the test is too brittle and would immediately break if a new code is added or if the API for ints were to be expanded.

Consider something like this:

self.assertTrue( {'value', 'name', 'description', 'phrase'} <= set(dir(HTTPStatus(404))))

Indeed my tests would break in that case, thanks for your review :)

I've made the following changes:

The HTTPStatus test is now in test_httplib.py, is it the right place?

two implemented tests are checking that dir() listing contains attributes

I have made the requested changes; please review again.

lem2clide · 2020-04-08T20:12:50Z

I have made the requested changes; please review again

bedevere-bot · 2020-04-08T20:12:53Z

Thanks for making the requested changes!

@rhettinger, @ethanfurman: please review the changes made to this pull request.

bedevere-bot · 2020-09-21T13:11:52Z

GH-22338 is a backport of this pull request to the 3.8 branch.

(cherry picked from commit 68526fe) Co-authored-by: Angelin BOOZ <[email protected]>

bedevere-bot · 2020-09-21T13:12:01Z

GH-22339 is a backport of this pull request to the 3.9 branch.

miss-islington · 2020-10-21T08:51:38Z

Thanks @lem2clide for the PR, and @ethanfurman for merging it 🌮🎉.. I'm working now to backport this PR to: 3.9.
🐍🍒⛏🤖

miss-islington · 2020-10-21T08:51:38Z

Thanks @lem2clide for the PR, and @ethanfurman for merging it 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

(cherry picked from commit 68526fe) Co-authored-by: Angelin BOOZ <[email protected]>

bedevere-bot · 2020-10-21T08:51:50Z

GH-22853 is a backport of this pull request to the 3.8 branch.

(cherry picked from commit 68526fe) Co-authored-by: Angelin BOOZ <[email protected]>

bedevere-bot · 2020-10-21T08:52:00Z

GH-22854 is a backport of this pull request to the 3.9 branch.

(cherry picked from commit 68526fe) Co-authored-by: Angelin BOOZ <[email protected]>

miss-islington · 2020-12-09T19:28:23Z

Thanks @lem2clide for the PR, and @ethanfurman for merging it 🌮🎉.. I'm working now to backport this PR to: 3.9.
🐍🍒⛏🤖

miss-islington · 2020-12-10T20:45:06Z

Thanks @lem2clide for the PR, and @ethanfurman for merging it 🌮🎉.. I'm working now to backport this PR to: 3.9.
🐍🍒⛏🤖

miss-islington · 2020-12-12T18:44:24Z

Thanks @lem2clide for the PR, and @ethanfurman for merging it 🌮🎉.. I'm working now to backport this PR to: 3.9.
🐍🍒⛏🤖

bedevere-bot · 2020-12-13T00:27:25Z

GH-23746 is a backport of this pull request to the 3.9 branch.

(cherry picked from commit 68526fe) Co-authored-by: Angelin BOOZ <[email protected]>

lem2clide requested a review from ethanfurman as a code owner March 29, 2020 20:56

the-knights-who-say-ni added the CLA not signed label Mar 29, 2020

bedevere-bot added the awaiting review label Mar 29, 2020

ethanfurman self-assigned this Mar 29, 2020

rhettinger requested changes Mar 30, 2020

View reviewed changes

bedevere-bot added awaiting changes and removed awaiting review labels Mar 30, 2020

dimaqq reviewed Mar 30, 2020

View reviewed changes

the-knights-who-say-ni added CLA signed and removed CLA not signed labels Mar 30, 2020

lem2clide force-pushed the bpo-40084-dir-attributes-listing branch from aceecf3 to 9b2cf1a Compare March 30, 2020 19:39

ethanfurman requested changes Mar 30, 2020

View reviewed changes

lem2clide force-pushed the bpo-40084-dir-attributes-listing branch 2 times, most recently from 13b3621 to 71ed596 Compare March 30, 2020 20:56

ethanfurman requested a review from rhettinger March 30, 2020 23:53

rhettinger reviewed Mar 31, 2020

View reviewed changes

bpo-40084: dir() includes entries from instance dict

03d064b

lem2clide force-pushed the bpo-40084-dir-attributes-listing branch from 71ed596 to 03d064b Compare April 1, 2020 00:06

bedevere-bot added awaiting change review and removed awaiting changes labels Apr 8, 2020

bedevere-bot requested review from ethanfurman and rhettinger April 8, 2020 20:12

rhettinger approved these changes Apr 15, 2020

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting change review labels Apr 15, 2020

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 21, 2020

bpo-40084: Enum - dir() includes member attributes (pythonGH-19219)

2521864

(cherry picked from commit 68526fe) Co-authored-by: Angelin BOOZ <[email protected]>

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 21, 2020

bpo-40084: Enum - dir() includes member attributes (pythonGH-19219)

bfb9eff

(cherry picked from commit 68526fe) Co-authored-by: Angelin BOOZ <[email protected]>

bedevere-bot removed the needs backport to 3.9 only security fixes label Sep 21, 2020

xzy3 pushed a commit to xzy3/cpython that referenced this pull request Oct 18, 2020

bpo-40084: Enum - dir() includes member attributes (pythonGH-19219)

b7dafa2

methane added needs backport to 3.8 needs backport to 3.9 only security fixes labels Oct 21, 2020

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 21, 2020

bpo-40084: Enum - dir() includes member attributes (pythonGH-19219)

6cd95d5

(cherry picked from commit 68526fe) Co-authored-by: Angelin BOOZ <[email protected]>

bedevere-bot removed the needs backport to 3.8 label Oct 21, 2020

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 21, 2020

bpo-40084: Enum - dir() includes member attributes (pythonGH-19219)

6ba5f46

(cherry picked from commit 68526fe) Co-authored-by: Angelin BOOZ <[email protected]>

bedevere-bot removed the needs backport to 3.9 only security fixes label Oct 21, 2020

ethanfurman pushed a commit that referenced this pull request Dec 9, 2020

bpo-40084: [Enum] dir() includes member attributes (GH-19219) (GH-22853)

f6d1520

(cherry picked from commit 68526fe) Co-authored-by: Angelin BOOZ <[email protected]>

ethanfurman added the needs backport to 3.9 only security fixes label Dec 9, 2020

ethanfurman added needs backport to 3.9 only security fixes and removed needs backport to 3.9 only security fixes labels Dec 10, 2020

ethanfurman added needs backport to 3.9 only security fixes and removed needs backport to 3.9 only security fixes labels Dec 12, 2020

bedevere-bot removed the needs backport to 3.9 only security fixes label Dec 13, 2020

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Dec 13, 2020

bpo-40084: Enum - dir() includes member attributes (pythonGH-19219)

6ef3aae

(cherry picked from commit 68526fe) Co-authored-by: Angelin BOOZ <[email protected]>

miss-islington added a commit that referenced this pull request Dec 14, 2020

bpo-40084: Enum - dir() includes member attributes (GH-19219)

33cbb04

(cherry picked from commit 68526fe) Co-authored-by: Angelin BOOZ <[email protected]>

		@@ -0,0 +1 @@
		Enum.__dir__ listing includes entries from instance dict

Uh oh!

bpo-40084: Enum.__dir__ listing includes entries from instance dict #19219

bpo-40084: Enum.__dir__ listing includes entries from instance dict #19219

Uh oh!

Conversation

lem2clide commented Mar 29, 2020 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

the-knights-who-say-ni commented Mar 29, 2020

CLA Missing

Uh oh!

rhettinger left a comment

Choose a reason for hiding this comment

Uh oh!

bedevere-bot commented Mar 30, 2020

Uh oh!

dimaqq Mar 30, 2020

Choose a reason for hiding this comment

Uh oh!

ethanfurman Mar 30, 2020

Choose a reason for hiding this comment

Uh oh!

lem2clide Mar 30, 2020

Choose a reason for hiding this comment

Uh oh!

ethanfurman left a comment

Choose a reason for hiding this comment

Uh oh!

ethanfurman Mar 30, 2020

Choose a reason for hiding this comment

Uh oh!

ethanfurman Mar 30, 2020

Choose a reason for hiding this comment

Uh oh!

lem2clide commented Mar 30, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rhettinger Mar 31, 2020

Choose a reason for hiding this comment

Uh oh!

lem2clide Apr 1, 2020

Choose a reason for hiding this comment

Uh oh!

lem2clide commented Apr 8, 2020

Uh oh!

bedevere-bot commented Apr 8, 2020

Uh oh!

bedevere-bot commented Sep 21, 2020

Uh oh!

bedevere-bot commented Sep 21, 2020

Uh oh!

miss-islington commented Oct 21, 2020

Uh oh!

miss-islington commented Oct 21, 2020

Uh oh!

bedevere-bot commented Oct 21, 2020

Uh oh!

bedevere-bot commented Oct 21, 2020

Uh oh!

miss-islington commented Dec 9, 2020

Uh oh!

miss-islington commented Dec 10, 2020

Uh oh!

miss-islington commented Dec 12, 2020

Uh oh!

bedevere-bot commented Dec 13, 2020

Uh oh!

Uh oh!

bpo-40084: Enum.dir listing includes entries from instance dict #19219

bpo-40084: Enum.dir listing includes entries from instance dict #19219

lem2clide commented Mar 29, 2020 •

edited by bedevere-bot

Loading

lem2clide commented Mar 30, 2020 •

edited

Loading