Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bpo-45535: Improve output of Enum dir() #29316

Merged
merged 13 commits into from
Dec 2, 2021

Conversation

AlexWaygood
Copy link
Member

@AlexWaygood AlexWaygood commented Oct 29, 2021

This PR modifies the EnumType.__dir__() and Enum.__dir__() to ensure
that user-defined methods and methods inherited from mixin classes always
show up in the output of help(). This change also makes it easier for
IDEs to provide auto-completion.

Apologies for the big diff on the tests.

https://bugs.python.org/issue45535

This PR modifies the ``EnumType.__dir__()`` and ``Enum.__dir__()`` to ensure
that user-defined methods and methods inherited from mixin classes always
show up in the output of `help()`. This change also makes it easier for
IDEs to provide auto-completion.

Apologies for the big diff on the tests.

>>> dir(Planet)
['EARTH', 'JUPITER', 'MARS', 'MERCURY', 'NEPTUNE', 'SATURN', 'URANUS', 'VENUS', '__class__', '__doc__', '__members__', '__module__']
['EARTH', 'JUPITER', 'MARS', 'MERCURY', 'NEPTUNE', 'SATURN', 'URANUS', 'VENUS', '__class__', '__doc__', '__init__', '__members__', '__module__', 'surface_gravity']
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does __init__ need to be here? It will never be called after the enum class is created.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

surface_gravity is here so completion works?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think having __init__ show up on dir(Planet) improves the output of help(Planet.EARTH). Without __init__ being present on dir(Planet), there is no reference in the output from help(Planet.EARTH) to the fact that the member has two named attributes, mass and radius. With __init__ being present on dir(Planet), however, the signature of the __init__ function shows up in the output of help(Planet.EARTH), so we can clearly see that the member has two named attributes, mass and radius.

self.assertNotIn('__format__', int_enum_dir)
self.assertNotIn('__hash__', int_enum_dir)

class OverridesFormatOutsideEnumModule(Enum):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we lose IDE or completion help by not listing __format__ in dir()?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, we don't, and I agree that __format__ should be excluded from the dir() of Enum, IntEnum and StrEnum, as well as any classes that inherit from these enum classes without overriding __format__. This is still the case in my PR.

But if a user is defining a custom __format__ method in their own Enum class, I think it would be surprising for the user if __format__ did not show up in dir() or help(). Their implementation of __format__ could have some unusual or surprising behaviour, which they might explain in detail in the docstring; but the docstring is of limited use if it doesn't show up in the output of help().

@github-actions
Copy link

github-actions bot commented Dec 2, 2021

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale Stale PR or inactive for long period of time. label Dec 2, 2021
@ethanfurman
Copy link
Member

I haven't forgotten. :-)

@ethanfurman
Copy link
Member

Alex, I'd like to accept and merge this PR and give you credit since the only reason __dir__ is being expanded is because you pushed for it. However, you may not see much of the actual code in the new enum.py file as I'm redoing a lot of the internals and test structure.

The other option is to close the PR, and simply add your name to the whatsnew in the improved __dir__ section.

I think the first option is better for your Github stats. :-)

Which option would you like to take?

@AlexWaygood
Copy link
Member Author

Alex, I'd like to accept and merge this PR and give you credit since the only reason __dir__ is being expanded is because you pushed for it. However, you may not see much of the actual code in the new enum.py file as I'm redoing a lot of the internals and test structure.

The other option is to close the PR, and simply add your name to the whatsnew in the improved __dir__ section.

I think the first option is better for your Github stats. :-)

Which option would you like to take?

Thank you, this is such a lovely message to get! It may seem silly, but could we possibly go for Option 1? I'm not currently employed in the tech sector (and am self-taught), but it's something I'd be very interested in potentially exploring in the future, so something like this would help boost my portfolio a little :)

@ethanfurman
Copy link
Member

Happy to. I mostly didn't want you to be disappointed in the future when you check the code base and the details don't match what is getting committed now.

@ethanfurman ethanfurman merged commit b2afdc9 into python:main Dec 2, 2021
@AlexWaygood
Copy link
Member Author

AlexWaygood commented Dec 2, 2021

Happy to. I mostly didn't want you to be disappointed in the future when you check the code base and the details don't match what is getting committed now.

Of course, I completely understand -- thanks for the heads-up!

@AlexWaygood AlexWaygood deleted the Enum-dir-improvements branch December 2, 2021 16:50
@bedevere-bot
Copy link

bedevere-bot commented Dec 2, 2021

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot ARM64 Windows Non-Debug 3.x has failed when building commit b2afdc9.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/730/builds/361) and take a look at the build logs.
  4. Check if the failure is related to this commit (b2afdc9) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/730/builds/361

Summary of the results of the build (if available):

Click to see traceback logs
remote: Enumerating objects: 16, done.        
remote: Counting objects:   6% (1/15)        
remote: Counting objects:  13% (2/15)        
remote: Counting objects:  20% (3/15)        
remote: Counting objects:  26% (4/15)        
remote: Counting objects:  33% (5/15)        
remote: Counting objects:  40% (6/15)        
remote: Counting objects:  46% (7/15)        
remote: Counting objects:  53% (8/15)        
remote: Counting objects:  60% (9/15)        
remote: Counting objects:  66% (10/15)        
remote: Counting objects:  73% (11/15)        
remote: Counting objects:  80% (12/15)        
remote: Counting objects:  86% (13/15)        
remote: Counting objects:  93% (14/15)        
remote: Counting objects: 100% (15/15)        
remote: Counting objects: 100% (15/15), done.        
remote: Compressing objects:   9% (1/11)        
remote: Compressing objects:  18% (2/11)        
remote: Compressing objects:  27% (3/11)        
remote: Compressing objects:  36% (4/11)        
remote: Compressing objects:  45% (5/11)        
remote: Compressing objects:  54% (6/11)        
remote: Compressing objects:  63% (7/11)        
remote: Compressing objects:  72% (8/11)        
remote: Compressing objects:  81% (9/11)        
remote: Compressing objects:  90% (10/11)        
remote: Compressing objects: 100% (11/11)        
remote: Compressing objects: 100% (11/11), done.        
remote: Total 16 (delta 4), reused 4 (delta 4), pack-reused 1        
From https://github.com/python/cpython
 * branch                  main       -> FETCH_HEAD
Note: switching to 'b2afdc95cc8f4e9228148730949a43cef0323f15'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by switching back to a branch.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -c with the switch command. Example:

  git switch -c <new-branch-name>

Or undo this operation with:

  git switch -

Turn off this advice by setting config variable advice.detachedHead to false

HEAD is now at b2afdc95cc [bpo-45535](https://bugs.python.org/issue45535): Improve output of Enum ``dir()`` (GH-29316)
Switched to and reset branch 'main'

Could Not Find C:\Workspace\buildarea\3.x.linaro-win-arm64.nondebug\build\Lib\*.pyc
The system cannot find the file specified.
Could Not Find C:\Workspace\buildarea\3.x.linaro-win-arm64.nondebug\build\PCbuild\python*.zip

Could Not Find C:\Workspace\buildarea\3.x.linaro-win-arm64.nondebug\build\Lib\*.pyc
The system cannot find the file specified.
Could Not Find C:\Workspace\buildarea\3.x.linaro-win-arm64.nondebug\build\PCbuild\python*.zip

@ethanfurman
Copy link
Member

I believe this is a false positive.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Stale PR or inactive for long period of time.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants