WIP: Optionally use pickle5

[jakirkham]

Fixes #179

Optionally uses pickle5 in cloudpickle.

[jakirkham]

Before doing more work here, would be curious to get people's thoughts on this approach 🙂

Does this generally seem reasonable? Or would we want to approach this in a different way?

[pierreglaser]

Before doing more work here, would be curious to get people's thoughts on this approach

The general approach (optionally rely on pickle5) looks good to me. We should also add/modify an entry in the CI to test cloudpickle against Python < 3.8 and pickle5. Thanks @jakirkham!

[codecov]

Codecov Report

Merging #364 into master will decrease coverage by 33.00%.
The diff coverage is 45.45%.

@@             Coverage Diff             @@
##           master     #364       +/-   ##
===========================================
- Coverage   92.95%   59.95%   -33.01%     
===========================================
  Files           2        3        +1     
  Lines         809      824       +15     
  Branches      164      168        +4     
===========================================
- Hits          752      494      -258     
- Misses         29      313      +284     
+ Partials       28       17       -11     

Impacted Files	Coverage Δ
cloudpickle/compat.py	30.00% <30.00%> (ø)
cloudpickle/cloudpickle_fast.py	93.72% <50.00%> (-2.01%)	⬇️
cloudpickle/cloudpickle.py	46.43% <100.00%> (-45.40%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 70dc1df...8c6518b. Read the comment docs.

[jakirkham]

Have run into some test issues around CellType (added in Python 3.8) and other related things. Not sure if there is an easy way to bypass these sorts of issues (since they are not the primary focus here). Thoughts? 🙂

[pierreglaser]

With regards to the types.CellType issue: cloudpickle_fast.py was originally written for Python 3.8+, so if we want to use it for Python < 3.8, we have no choice but to make the reducers within cloudpickle_fast.py cross-version.

The clean way though would be to refactor the organization of the reducers in cloudpickle, many of which are duplicated between cloudpickle.py and cloudpickle_fast.py, so that both modules import the same ones. See #284. Maybe this PR is a good occasion to do this (within this PR or separately) :)

[ogrisel]

If we refactor the reducers, we should make a best effort to still make it possible to unpickle older pickles.

Even if this is not part of the cloudpickle public API (see the diclaimer for the scope of the project in the README) it would be great to preserve backward compat with previous version pickles if not too costly from a maintenance / tech debt point of view.

[pierreglaser]

That is a very good remark @ogrisel. Would the "deprecation" cycle I suggested in #359 (comment) work for you? IMHO it achieves a good tradeoff between all the stakes (code complexity, user friendliness, contract of cloudpickle...)

[ogrisel]

+1 for deprecation warnings when possible.

[jakirkham]

Closing as this has been superseded by PR ( #370 ).