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

Some cleanups, fix memoryview support #122

Merged
merged 5 commits into from
Oct 27, 2017

Conversation

pitrou
Copy link
Member

@pitrou pitrou commented Oct 26, 2017

A couple assorted cleanups:

  • save_global can use the default implementation most of the time
  • save_buffer and save_memory needn't call Pickler.save_string directly
  • use save_reduce instead of dumping opcodes by hand where possible
  • remove Pandas from CI setup since it's not used anywhere in the test suite

Also, memoryviews should obviously serialize as bytes, not str.

@codecov-io
Copy link

codecov-io commented Oct 26, 2017

Codecov Report

Merging #122 into master will decrease coverage by 2.37%.
The diff coverage is 90%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #122      +/-   ##
==========================================
- Coverage   86.32%   83.94%   -2.38%     
==========================================
  Files           2        2              
  Lines         541      517      -24     
  Branches       98       90       -8     
==========================================
- Hits          467      434      -33     
- Misses         55       62       +7     
- Partials       19       21       +2
Impacted Files Coverage Δ
cloudpickle/cloudpickle.py 83.85% <90%> (-2.4%) ⬇️

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 2da4c24...c91aaf1. Read the comment docs.

@pitrou
Copy link
Member Author

pitrou commented Oct 26, 2017

Predictably, Python 2.6 fails on memoryview. Should I fix this PR or should we ditch 2.6 (and 3.3)? :-)
@rgbkrk

@rgbkrk
Copy link
Member

rgbkrk commented Oct 26, 2017

How about we finish making the release for v0.4.1 since it's still Python 2.6 compatible, then we merge this in noting that we'll drop 2.6 and 3.3 support for v0.5.0

@pitrou
Copy link
Member Author

pitrou commented Oct 26, 2017

Works for me :-)

@rgbkrk
Copy link
Member

rgbkrk commented Oct 26, 2017

I'm going to give @ssanderson a day or so to make the release then I'll ping again and go from there. 😉

@hugovk
Copy link
Contributor

hugovk commented Oct 26, 2017

+1 to dropping 2.6 and 3.3.

Here's the pip installs for cloudpickle from PyPI for the last month -- they have very small numbers!

$ pypinfo --percent --pip cloudpickle pyversion
python_version percent download_count
-------------- ------- --------------
3.5              54.4%         41,113
2.7              30.5%         23,082
3.6              13.0%          9,834
3.4               1.9%          1,453
3.3               0.1%            103
3.7               0.1%             41
2.6               0.0%             12

@rgbkrk
Copy link
Member

rgbkrk commented Oct 26, 2017

We have more Python3 downloads than Python2 downloads?!?! 😮

@mrocklin
Copy link
Contributor

mrocklin commented Oct 26, 2017 via email

@rgbkrk
Copy link
Member

rgbkrk commented Oct 26, 2017

Alright, adjust that Travis matrix to get rid of 2.6 support and we can bring this in.

@pitrou pitrou merged commit 9bc424d into cloudpipe:master Oct 27, 2017
@pitrou pitrou deleted the save_global_cleanup branch October 27, 2017 10:27
This was referenced Oct 28, 2017
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 <cutlerb@gmail.com>

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.

5 participants