GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — bpo-45756: Fix mock triggers dynamic lookup via the descriptor protocol. by hongweipeng · Pull Request #31348 · python/cpython

Conversation

@hongweipeng

@hongweipeng hongweipeng commented Feb 15, 2022 •
edited by bedevere-bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

https://bugs.python.org/issue45756

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@hongweipeng
Fix mock triggers dynamic lookup via the descriptor protocol.
45a99ec
@hongweipeng
hongweipeng requested a review from cjw296 as a code owner February 15, 2022 06:36
@bedevere-bot bedevere-bot added the awaiting review label Feb 15, 2022
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Feb 15, 2022
sobolevn
sobolevn reviewed Feb 15, 2022
View reviewed changes

@sobolevn sobolevn left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Isn't it the same as #29901 ?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@hongweipeng

hongweipeng commented Feb 15, 2022

Copy link
Copy Markdown
Contributor Author

Isn't it the same as #29901 ?

I think this is a good example of using inspect.getmembers_static.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@hongweipeng
simplify
e70c22f
@hongweipeng

hongweipeng commented Feb 15, 2022

Copy link
Copy Markdown
Contributor Author

Isn't it the same as #29901 ?

No, it only skip property in #29901 .

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@hongweipeng

hongweipeng commented Feb 15, 2022

Copy link
Copy Markdown
Contributor Author

This PR is similar to #22209 .

I check the difference between inspect.getmembers_static(obj) and for k in dir(obj) ... inspect.getattr_static(obj, k) .

I found that they behave differently when __dir__ is defined and the attribute is not exist.

class A:
    def __dir__(self):
        return ['__class__', 'no_member']

obj = A()
inspect.getmembers_static(obj)      # ['__class__']
[(k, inspect.getattr_static(obj, k)) for k in dir(obj)] # AttributeError

I don't know which behavior is more reasonable.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

JelleZijlstra
JelleZijlstra reviewed May 3, 2022
View reviewed changes
Comment thread Lib/unittest/mock.py
for attr in dir(spec):
if iscoroutinefunction(getattr(spec, attr, None)):
_spec_asyncs.append(attr)
for attr, _ in inspect.getmembers_static(spec, iscoroutinefunction):

@JelleZijlstra JelleZijlstra May 3, 2022

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

What about async classmethods? Would those still get recognized as coroutines?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@kjamieson kjamieson mannequin mentioned this pull request Apr 10, 2022
mock raises exception when using a spec with an attribute that raises exception on access #89917
Closed
@ezio-melotti ezio-melotti removed the CLA signed label Jul 13, 2022
@f0k f0k mentioned this pull request Feb 22, 2023
unittest.mock spec calls class properties #85934
Closed
@carljm

carljm commented May 5, 2023

Copy link
Copy Markdown
Member

Thanks for the PR! I think we should consolidate the discussion to #22209. If we do this, we should prefer continuing to use dir and using getattr_static (as done in #22209), rather than switching to getmembers_static, because switching away from dir causes extra avoidable backward-incompatibility.

Also, I think the async-classmethod case is not fixable using getmembers_static, but is fixable via getattr_static.

Closing as duplicate of #22209.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@carljm carljm closed this May 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JelleZijlstra JelleZijlstra JelleZijlstra left review comments
@sobolevn sobolevn sobolevn left review comments
@cjw296 cjw296 Awaiting requested review from cjw296 cjw296 is a code owner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

awaiting review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

7 participants

@hongweipeng @carljm @JelleZijlstra @sobolevn @the-knights-who-say-ni @ezio-melotti @bedevere-bot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.