◐ Shell
reader mode source ↗
Skip to content

bpo-16251: pickle special methods and custom __getattr__#3508

Closed
ambv wants to merge 1 commit into
python:masterfrom
ambv:bpo-16251
Closed

bpo-16251: pickle special methods and custom __getattr__#3508
ambv wants to merge 1 commit into
python:masterfrom
ambv:bpo-16251

Conversation

@ambv

@ambv ambv commented Sep 12, 2017

Copy link
Copy Markdown
Contributor

This PR doesn't change the lookup to types as the issue's title on BPO
suggests. As Antoine and Raymond point out, there'd be significant danger of
breaking existing code.

Instead, we're doing the following:

  • introducing a new function in copyreg called getcallable() that mimics
    getattr() but swallows all exceptions (like hasattr() on Python 2) and
    does some basic callability tests;

  • switching all relevant usage of hasattr() and getattr() in copy and
    copyreg to use getcallable() instead.

This fixes two common failure scenarios covered at length in bpo-16251, its
duplicates, as well as on StackOverflow, etc. Worth fixing IMHO.

Funnily enough, the recursion limit exceeded scenario happened to work on
Python 2 since hasattr() there swallowed the RecursionError.

https://bugs.python.org/issue16251

This PR doesn't change the lookup to types as the issue's title on BPO
suggests.  As Antoine and Raymond suggest, there'd be significant danger of
breaking existing code.

Instead, we're doing the following:

* introducing a new function in `copyreg` called `getcallable()` that mimics
`getattr()` but swallows *all* exceptions (like `hasattr()` on Python 2) and
does some basic callability tests;

* switching all relevant usage of `hasattr()` and `getattr()` in `copy` and
`copyreg` to use `getcallable()` instead.

This fixes two common failure scenarios covered at length in bpo-16251, its
duplicates, as well as on StackOverflow, etc.  Worth fixing IMHO.

Funnily enough, the recursion limit exceeded scenario happened to work on
Python 2 since `hasattr()` there swallowed the RecursionError.
@ambv

ambv commented Sep 12, 2017

Copy link
Copy Markdown
Contributor Author

I'll add the Blurb entry after tests pass and the patch gets accepted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants