[BUG] Python implementation pickles differently than native implementation

[apmorton]

OS version: Ubuntu 22.04.3 LTS
Python3 version: Python 3.11.6 | packaged by conda-forge | (main, Oct 3 2023, 10:40:35) [GCC 12.3.0]

In [4]: pickle.dumps(frozendict.frozendict(a=1), protocol=0)
Out[4]: b'cfrozendict.core\nfrozendict\np0\n((dp1\nVa\np2\nI1\nstp3\nRp4\n.'

vs

In [10]: pickle.dumps(frozendict.frozendict(a=1), protocol=0)
Out[10]: b'cfrozendict\nfrozendict\np0\n((dp1\nVa\np2\nI1\nstp3\nRp4\n.'

This results in using the python implementation when loading a pickle from a version of python that does not support the native extension in a version of python that does support it.

A possible solution to this would be to change the __module__ of the native implementation:

frozendict.__module__ = 'frozendict'

This is unfortunately hard to test, as doing it in core.py causes many tests to fail with:

_pickle.PicklingError: Can't pickle <class 'frozendict.frozendict'>: it's not the same object as frozendict.frozendict

This is somewhat of a false-positive since the test suite runs on a python interpreter that does have the native extension but is intentionally using the python implementation, and thus frozendict.frozendict really isn't frozendict.core.frozendict - but this is an unrealistic situation.

[Marco-Sulla]

This results in using the python implementation when loading a pickle from a version of python that does not support the native extension in a version of python that does support it.

Good point, but I suppose it's too late.

Changing now the module name of the pure py implementation will break more things than fixing them.

I close the issue as wontfix, but feel free to reply. If you find a retrocompatible solution I'll be happy to implement it.

[apmorton]

Good point, but I suppose it's too late.

I don't think it should be too late - this is a pretty surprising behavior and should be fixed.
It seems the intent of frozendict.core.frozendict is ONLY to be used when the native extension is unavailable, and thus it should behave identically. Do you disagree?

Other than the tests, which can be fixed, I can't find any evidence of this change breaking externally visible behavior.

# still able to load a pickle that mentions core
pickle.loads(b'cfrozendict.core\nfrozendict\np0\n((dp1\nVa\np2\nI1\nstp3\nRp4\n.')

[Marco-Sulla]

I can't find any evidence of this change breaking externally visible behavior.

Mh, maybe I was too cryptic in my reply. The problem is retrocompatibility.

What if someone used the pure py implementation and stored the pickles? They will be no more able to load them.

IMHO this scenario is more common than the case someone uses two versions of Python, one to pickle and one to unpickle. This is why I said "will break more things than fixing them".

[apmorton]

What if someone used the pure py implementation and stored the pickles? They will be no more able to load them.

This is not true - as I showed in my last reply - files already pickled from the pure python implementation that mention frozendict.core will still depickle when __module__ is modified to say frozendict _so long as the code itself still lives in frozendict.core.

The only change is that newly pickled files will say frozendict. (and those files correctly depickle in all cases).

[Marco-Sulla]

Mh. I'm a bit reluctant to touch __module__.

Are you completely sure that touching it and running the tests against the pure py implementation, no error comes out?

I reopen the issue.

[Marco-Sulla]

(I would add that I wanted to reopen this issue before your comment, because it came in my mind that when God will give me time to fix #68, then the regression will raise anyway)

[Marco-Sulla]

As mentioned by you, this could fix #88