-
Notifications
You must be signed in to change notification settings - Fork 176
Compatibility: expose cloudpickle.CloudPickler also as cloudpickle.Pickler #392
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
Conversation
Codecov Report
@@ Coverage Diff @@
## master #392 +/- ##
=======================================
Coverage 91.61% 91.61%
=======================================
Files 3 3
Lines 656 656
Branches 135 135
=======================================
Hits 601 601
Misses 34 34
Partials 21 21 Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, Thanks @twoertwein. This also needs a changelog entry - can you write one?
Regarding the README
, issue, I think that at some point we should make it clear that accessing cloudpickle.Pickler
is now the recommended way to use cloudpickle
's Pickler
subclass. But it is not urgent, so I won't mind if we leave the README
as it is for now.
Co-authored-by: Pierre Glaser <[email protected]>
Alright, let's merge! Thank you very much @twoertwein. |
I'll do a release of |
Some people/projects expect that a module implementing a Pickler, provides the Pickler class as
<module>.Pickler
, for example pytorch/pytorch#38098.Since the current version of Cloudpickler doesn't define
cloudpickle.Pickler
, let's definecloudpickle.Pickler
ascloudpickle.CloudPickler
.Fixes #366