set pickling problems

[d1b222cf-9828-419b-858a-f606924f31ac]

BPO	1062353
Nosy	@rhettinger
Files	set.diff: Diff and tests

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields: ```python assignee = None closed_at = created_at = labels = ['extension-modules'] title = 'set pickling problems' updated_at = user = 'https://bugs.python.org/ddorfman' ``` bugs.python.org fields: ```python activity = actor = 'ddorfman' assignee = 'none' closed = True closed_date = None closer = None components = ['Extension Modules'] creation = creator = 'ddorfman' dependencies = [] files = ['6359'] hgrepos = [] issue_num = 1062353 keywords = ['patch'] message_count = 3.0 messages = ['47275', '47276', '47277'] nosy_count = 2.0 nosy_names = ['rhettinger', 'ddorfman'] pr_nums = [] priority = 'normal' resolution = 'wont fix' stage = None status = 'closed' superseder = None type = None url = 'https://bugs.python.org/issue1062353' versions = [] ```

[d1b222cf-9828-419b-858a-f606924f31ac]

As with deque (SF bpo-1062279), two problems with set.__reduce__:

1. Recursive sets (which can be constructed with the aid of a hashable mutable object) aren't pickled correctly because __reduce wants a reference to itself in the call to its constructor. Fix by moving the keys to the state argument and resurrecting them in __setstate (test_pickling_recursive).

2. Without the standard reduce, we have to take care of the instance dictionary ourselves. The test for this is in a new TestSubclassOps class that is mixed in to TestSetSubclass and TestFrozenSetSubclass. I'm not sure if such a mixin is the best way to distribute that test.

The biggest drawback to this patch is that __setstate__ makes it possible to mutate a frozenset. This implementation clears the cached hash after such a mutation, but even then it can be used to cause havoc in dicts. Such havoc isn't fatal (this doesn't do anything that a regular class can't do), but it can be confusing. Not being able to do this was a desirable property of frozenset, but it's unlikely to happen on accident, and sets.ImmutableSet has surived without it. Unless one of the pickle gurus provides a better alternative to SF bpo-1062277, this might be the best option.

[rhettinger]

Logged In: YES user_id=80475

Am adding the dict argument so that subclass dictionaries are handled without extra coding. See Objects/setobject.c 1.31

Sets were designed to be non-recursive. While you can create shennanigans to introduce hashable mutable objects to be stored recursively, I have no interest in building support for them. Certainly, it is not worth introducing other anomalies or worth compilcating the code.

[d1b222cf-9828-419b-858a-f606924f31ac]

Logged In: YES user_id=908995

Fair enough. If sets weren't meant to be recursive then 1. 31 is sufficient.