FIX pickling for ABC in python3.7#189
Conversation
Codecov Report
@@ Coverage Diff @@
## master #189 +/- ##
==========================================
+ Coverage 84.09% 84.38% +0.28%
==========================================
Files 1 1
Lines 547 557 +10
Branches 99 104 +5
==========================================
+ Hits 460 470 +10
Misses 63 63
Partials 24 24
Continue to review full report at Codecov.
|
tomMoral
force-pushed
the
FIX_abc_reduce
branch
from
5acea3a to
654ea89
Compare
tomMoral
force-pushed
the
FIX_abc_reduce
branch
2 times, most recently
from
d485714 to
8fdbe68
Compare
tomMoral
force-pushed
the
FIX_abc_reduce
branch
2 times, most recently
from
fd506e6 to
c917956
Compare
|
The trick to get an up-to-date python3.7 release was given here! |
ogrisel
left a comment
ogrisel
left a comment
There was a problem hiding this comment.
Some comments but other than that LGTM. Thanks for the fix!
|
|
||
| AbstractClass.register(tuple) | ||
|
|
||
| depickled_base = pickle_depickle(AbstractClass, protocol=self.protocol) |
There was a problem hiding this comment.
This introduces a side effect in the test. I don't think it's possible to unregister tuple. Cloud you please instead use a new locally defined class, e.g.:
class SomeOtherClass(object):
pass
AbstractClass.register(SomeOtherClass)
...There was a problem hiding this comment.
Actually, it should also be possible to do:
AbstractClass.register(tuple)
try:
...
finally:
AbstractClass._abc_registry_clear()do as you wish.
There was a problem hiding this comment.
We would also need to clean the registry of depickled_base. Better not register tuple at all.
cloudpickle/cloudpickle.py
Outdated
| clsdict = dict(obj.__dict__) # copy dict proxy to a dict | ||
| clsdict.pop('__weakref__', None) | ||
|
|
||
| # For ABCMeta in python3.7, remove _abc_impl as it is not picklable. |
cloudpickle/cloudpickle.py
Outdated
| if "_abc_impl" in clsdict: | ||
| import abc | ||
| (registry, _, _, _) = abc._get_dump(obj) | ||
| clsdict["_abc_impl"] = [wr() for wr in registry] |
|
Thanks! Merged. However, I forgot to ask about adding an entry to the changelog. Can you please open a new PR for that? |
3 participants
Fix #180
The issue was that the
abcmodule have been entirely re-written in C. (python/cpython#5273)The
registermechanism is not accessible anymore. The proposed implementation simply pickle the registered classes usingabc._get_dumpand then re-register them. This does not conserve thecachebut this does not seem to be an issue (it is only a matter of performance for the first call toissubclass).