Skip to content

Fix python implementation pickling#91

Merged
Marco-Sulla merged 7 commits intoMarco-Sulla:masterfrom
apmorton:am/fix-module
Nov 28, 2023
Merged

Fix python implementation pickling#91
Marco-Sulla merged 7 commits intoMarco-Sulla:masterfrom
apmorton:am/fix-module

Conversation

@apmorton
Copy link
Copy Markdown
Contributor

@apmorton apmorton commented Nov 24, 2023

This PR is split into two commits for easier review since git doesn't correctly show the rename operation on core.py 😞 .

The first moved the pure python implementation to _frozendictpy so that core.py can become a compatibility wrapper, but makes no other changes. This allows existing pickles that mention frozendict.core.frozendict to correctly depickle as the native implementation in the future on python 3.11.

The second implements the fix for #87.

@Marco-Sulla
Copy link
Copy Markdown
Owner

Excuse me, but why you have renamed also core.py? Fixing #87 was not sufficient?

PS: I added a build and test for the pure py implementation in the pipeline, so I'm going to remove the pure py tests from the C extension tests.

Wait for it.

@apmorton
Copy link
Copy Markdown
Contributor Author

Excuse me, but why you have renamed also core.py?

I explained this in the PR description.

Existing files that have been pickled on python 3.11 mention frozendict.core.frozendict.
This means that if/when 3.11 gets a native extension these files will still depickle to the pure python implementation and you can end up with mixed native/python instances in a single process.

Moving the python implementation and setting up frozendict.core.frozendict to be an alias for frozendict.frozendict resolves this issue.

@Marco-Sulla
Copy link
Copy Markdown
Owner

This means that if/when 3.11 gets a native extension these files will still depickle to the pure python implementation and you can end up with mixed native/python instances in a single process.

Ok, but what if I pickle frozendict._frozendictpy.frozendict and then I switch to the C extension?

At this point, is it not much simpler to remove core.py?

@apmorton
Copy link
Copy Markdown
Contributor Author

Ok, but what if I pickle frozendict._frozendictpy.frozendict and then I switch to the C extension?

If you pickle frozendict._frozendictpy.frozendict it will appear in the pickle file as frozendict.frozendict because it has __module__ set to frozendict and depickle correctly in the future using the C extension.

@Marco-Sulla Marco-Sulla merged commit 60211ac into Marco-Sulla:master Nov 28, 2023
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.

2 participants