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

set pickling problems #41152

Closed
ddorfman mannequin opened this issue Nov 8, 2004 · 3 comments
Closed

set pickling problems #41152

ddorfman mannequin opened this issue Nov 8, 2004 · 3 comments
Labels
extension-modules C modules in the Modules dir

Comments

@ddorfman
Copy link
Mannequin

ddorfman mannequin commented Nov 8, 2004

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:

    assignee = None
    closed_at = <Date 2004-11-09.09:03:11.000>
    created_at = <Date 2004-11-08.11:07:49.000>
    labels = ['extension-modules']
    title = 'set pickling problems'
    updated_at = <Date 2004-11-09.09:03:11.000>
    user = 'https://bugs.python.org/ddorfman'

    bugs.python.org fields:

    activity = <Date 2004-11-09.09:03:11.000>
    actor = 'ddorfman'
    assignee = 'none'
    closed = True
    closed_date = None
    closer = None
    components = ['Extension Modules']
    creation = <Date 2004-11-08.11:07:49.000>
    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 = []

    @ddorfman
    Copy link
    Mannequin Author

    ddorfman mannequin commented Nov 8, 2004

    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.

    @ddorfman ddorfman mannequin closed this as completed Nov 8, 2004
    @ddorfman ddorfman mannequin added the extension-modules C modules in the Modules dir label Nov 8, 2004
    @ddorfman ddorfman mannequin closed this as completed Nov 8, 2004
    @ddorfman ddorfman mannequin added the extension-modules C modules in the Modules dir label Nov 8, 2004
    @rhettinger
    Copy link
    Contributor

    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.

    @ddorfman
    Copy link
    Mannequin Author

    ddorfman mannequin commented Nov 9, 2004

    Logged In: YES
    user_id=908995

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

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 9, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    extension-modules C modules in the Modules dir
    Projects
    None yet
    Development

    No branches or pull requests

    1 participant