Skip to content

[MRG][BRANCH-0.4.x] Regression on pickling classes from the __main__ module #149

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

Merged
merged 11 commits into from
Feb 6, 2018

Conversation

HyukjinKwon
Copy link
Member

This PR backports #132 to branch-0.4.x and fixes #131.

@HyukjinKwon HyukjinKwon changed the title [MRG][BRANCH-0.4.x] Regression on pickling classes from the __main__ module [WIP][MRG][BRANCH-0.4.x] Regression on pickling classes from the __main__ module Feb 5, 2018
@HyukjinKwon HyukjinKwon changed the title [WIP][MRG][BRANCH-0.4.x] Regression on pickling classes from the __main__ module [MRG][BRANCH-0.4.x] Regression on pickling classes from the __main__ module Feb 5, 2018
@HyukjinKwon
Copy link
Member Author

Seems the test failure is by #151.


cloned = subprocess_pickle_echo(f4)
assert cloned(2) == f4(2)
"""
Copy link
Member Author

Choose a reason for hiding this comment

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

I removed protocol comparing to the original

@@ -28,9 +29,13 @@ def subprocess_pickle_echo(input_data):

"""
pickled_input_data = dumps(input_data)
Copy link
Member Author

Choose a reason for hiding this comment

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

ditto

@rgbkrk
Copy link
Member

rgbkrk commented Feb 5, 2018

Since I merged the other, I think you can rebase this branch now.

@codecov-io
Copy link

codecov-io commented Feb 6, 2018

Codecov Report

Merging #149 into 0.4.x will decrease coverage by 0.31%.
The diff coverage is 33.33%.

Impacted file tree graph

@@            Coverage Diff            @@
##           0.4.x     #149      +/-   ##
=========================================
- Coverage   83.9%   83.58%   -0.32%     
=========================================
  Files          1        1              
  Lines        528      530       +2     
  Branches      96       97       +1     
=========================================
  Hits         443      443              
- Misses        63       64       +1     
- Partials      22       23       +1
Impacted Files Coverage Δ
cloudpickle/cloudpickle.py 83.58% <33.33%> (-0.32%) ⬇️

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 22af562...3cfc303. Read the comment docs.

@rgbkrk rgbkrk merged commit 216e841 into cloudpipe:0.4.x Feb 6, 2018
HyukjinKwon pushed a commit to HyukjinKwon/spark that referenced this pull request Mar 8, 2018
## What changes were proposed in this pull request?

The version of cloudpickle in PySpark was close to version 0.4.0 with some additional backported fixes and some minor additions for Spark related things.  This update removes Spark related changes and matches cloudpickle [v0.4.3](https://github.com/cloudpipe/cloudpickle/releases/tag/v0.4.3):

Changes by updating to 0.4.3 include:
* Fix pickling of named tuples cloudpipe/cloudpickle#113
* Built in type constructors for PyPy compatibility [here](cloudpipe/cloudpickle@d84980c)
* Fix memoryview support cloudpipe/cloudpickle#122
* Improved compatibility with other cloudpickle versions cloudpipe/cloudpickle#128
* Several cleanups cloudpipe/cloudpickle#121 and [here](cloudpipe/cloudpickle@c91aaf1)
* [MRG] Regression on pickling classes from the __main__ module cloudpipe/cloudpickle#149
* BUG: Handle instance methods of builtin types cloudpipe/cloudpickle#154
* Fix <span>#</span>129 : do not silence RuntimeError in dump() cloudpipe/cloudpickle#153

## How was this patch tested?

Existing pyspark.tests using python 2.7.14, 3.5.2, 3.6.3

Author: Bryan Cutler <[email protected]>

Closes apache#20373 from BryanCutler/pyspark-update-cloudpickle-42-SPARK-23159.
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.

4 participants