Navigation Menu

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

Support converting sets.Set() objects #25

Merged
merged 9 commits into from Jul 30, 2019
Merged

Support converting sets.Set() objects #25

merged 9 commits into from Jul 30, 2019

Conversation

mgedmin
Copy link
Member

@mgedmin mgedmin commented Jul 25, 2019

Fixes #23.

This PR could use some work:

  • tests!
  • it doesn't work when run on Python 3
  • it produces pickles refering to __builtin__.set instead of builtins.set

@mgedmin
Copy link
Member Author

mgedmin commented Jul 25, 2019

NB: I noticed that the default renamer of '__builtin__ set': 'builtins set' causes calls to ZODB.broken.rebuild() to be embedded in the output pickles, if you run the conversion on Python 2. I don't think that's desired. I've added a commented-out test, but I'm not sure how to fix it. (Stuff a fake builtins module into sys.modules?)

@mgedmin
Copy link
Member Author

mgedmin commented Jul 29, 2019

I've found a way (an ugly one) of pickling sets so they unpickle correctly on Python 3 without relying on ZODB.broken.

@mgedmin mgedmin changed the title WIP: support converting sets.Set() objects Support converting sets.Set() objects Jul 29, 2019
@mgedmin
Copy link
Member Author

mgedmin commented Jul 29, 2019

I think this PR is ready for review.

Copy link
Member

@icemac icemac left a comment

Choose a reason for hiding this comment

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

LGTM from reading through the code.

@mgedmin mgedmin merged commit 8f257ec into master Jul 30, 2019
@mgedmin mgedmin deleted the sets-set branch July 30, 2019 12:53
@mgedmin
Copy link
Member Author

mgedmin commented Jul 30, 2019

@icemac: I would like to request a PyPI release with this fix.

I'm happy to help with making the release if you add me (PyPI username mgedmin) as a PyPI maintainer for zodbupdate.

@icemac
Copy link
Member

icemac commented Jul 30, 2019

@mgedmin I added you as owner of this package on PyPI.

@mgedmin
Copy link
Member Author

mgedmin commented Jul 30, 2019

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ideas for migrating sets.Set() instances to builtins.set() during py3 migration?
2 participants