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

cloudpickle should import Pickler class directly from the pickle module #1238

Closed
jvesely opened this issue Nov 2, 2021 · 4 comments
Closed

Comments

@jvesely
Copy link

jvesely commented Nov 2, 2021

from _pickle import Pickler # noqa: F401

uses _pickle module to import Pickler. However, as per python 3.8 documentation[0] this class is available directly in the pickle module.

from pickle import Pickler

works just fine.

[0] https://docs.python.org/3.8/library/pickle.html#pickle.Pickler

@jvesely
Copy link
Author

jvesely commented Nov 2, 2021

This also fixes issues with pypy3:


    import sys
    
    
    if sys.version_info < (3, 8):
        try:
            import pickle5 as pickle  # noqa: F401
            from pickle5 import Pickler  # noqa: F401
        except ImportError:
            import pickle  # noqa: F401
            from pickle import _Pickler as Pickler  # noqa: F401
    else:
        import pickle  # noqa: F401
>       from _pickle import _Pickler as Pickler  # noqa: F401
E       ModuleNotFoundError: No module named '_pickle'

../pypy38-venv/lib/pypy3.8/site-packages/joblib/externals/cloudpickle/compat.py:13: ModuleNotFoundError

@tomMoral
Copy link
Contributor

tomMoral commented Nov 3, 2021

cloudpickle is vendored in joblib (an exact copy of its latest version is included in external). The issue tracker for its development is here: https://github.com/cloudpipe/cloudpickle, so you should report issues relative to it there.

That being said, I think this like this here for historical reasons. From what I understand, _pickle.Pickler is a fast C implementation that could not be customized prior to 3.8, while pickle._Pickler is a python implementation that can be slow but is customizable. That is why cloudpickle tries to import the former explicitly. The cloudpickle maintainers will be able to better tell you if doing the change you propose might have performance implication.

@tomMoral tomMoral closed this as completed Nov 3, 2021
@jvesely
Copy link
Author

jvesely commented Nov 3, 2021

thank you.
reported as cloudpipe/cloudpickle#458

note that the code path is specific to 3.8+, _pickle.Pickler and pickle.Pickler should be equivalent (see the above-posted issue)

@jvesely
Copy link
Author

jvesely commented May 24, 2022

The issue on the cloudpickle side (cloudpipe/cloudpickle#458) has been fixed in cloudpipe/cloudpickle#469.

How long does it usually take for cloudpickle changes to propagate to joblib?

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

No branches or pull requests

2 participants