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.

Sign up for GitHub

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

Jump to bottom

Allow 3.7 Pickles to be Loaded in 3.8 #406

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Allow 3.7 Pickles to be Loaded in 3.8 #406

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ec96fce
Use _create_code Logic when loading Pickles
njwhite Mar 12, 2021
0b7fd9a
add tests case
njwhite Jun 1, 2021
125a3a8
Merge branch 'master' of github.com:uqfoundation/dill
njwhite Jun 1, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Binary file added tests/lambda.pkl
View file
Open in desktop
Binary file not shown.
10 changes: 10 additions & 0 deletions tests/test_functions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
# - https://github.com/uqfoundation/dill/blob/master/LICENSE

import dill
import os
import sys
dill.settings['recurse'] = True

Expand All @@ -30,6 +31,14 @@ def function_d(d, d1, d2=1):
return d + d1 + d2


def lambda_a():
pkl = os.path.join(
os.path.dirname(__file__),
"lambda.pkl")
Copy link
Member

Choose a reason for hiding this comment

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

It would be much better to write the file contents elsewhere, instead of relying on a stored pickle file. Was the file written in python 3.7? dill tests are currently run with 2.7, 3.6, 3.7, 3.8, 3.9, 3.10, pypy27, pypy36, and pypy37. Is the test only supposed to run with 3.8?

You don't need to add a test case into the code at the moment, if it's difficult. Rather, just present the details in the main conversation of the Github issue, so I and others can reproduce what you are seeing.

Copy link
Author

Choose a reason for hiding this comment

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

The file was written with python 3.7 / dill 0.3.0. I think that’s why you can’t put the file contents there (and need the binary) - the bug seems to be that dill isn’t backwards compatible.

The pickled object is just lambda x: x.

Copy link
Member

Choose a reason for hiding this comment

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

Here's an example using lambda in dill master with 3.7:

Python 3.7.10 (default, Mar 18 2021, 06:11:04) 
[Clang 9.0.0 (clang-900.0.39.2)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import dill
>>> dill.dump(lambda x:x, open('test.pkl', 'wb'))

and loading with 3.8...

Python 3.8.10 (default, May  7 2021, 23:18:56) 
[Clang 9.0.0 (clang-900.0.39.2)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import dill
>>> f = dill.load(open('test.pkl', 'rb'))
>>> f(4)
4
>>>

What is your PR doing that is not possible currently?

Copy link
Author

Choose a reason for hiding this comment

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

You need to dump with dill 0.3.0 and python 3.7, not master/3.7 to reproduce.

Copy link
Member

Choose a reason for hiding this comment

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

So... you are saying that the issue is that old pickles from python 3.7 created with dill 0.3.0 don't unpickle in python 3.8 with dill master. I'm assuming this is also the case for other old versions of dill (before _create_function was recently modified).

Am I correct in thinking that you could, as a workaround, load the pickle in 3.7 with dill master, and then dump it again... then the resulting file would be able to be opened with 3.8?

Copy link
Author

Choose a reason for hiding this comment

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

My 3.8 workaround is just to set:

dill._dill._reverse_typemap[‘CodeType’] = dill._dill._create_code

much easier than re-serialising all the pickle files I have lying around :)

Copy link
Member

Choose a reason for hiding this comment

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

Riiight, of course... hence the PR. I never really considered it, however, I'm wonder if adding other of the _create_ functions to the reverse_typemap is worth investigating. I'm not certain of what functionality it might impact.

with open(pkl,"rb") as fh:
fn = dill.load(fh)
assert fn is not None

if is_py3():
exec('''
def function_e(e, *e1, e2=1, e3=2):
Expand Down Expand Up @@ -65,3 +74,4 @@ def test_functions():

if __name__ == '__main__':
test_functions()
lambda_a()