Opened 10 years ago
Closed 6 years ago
#10962 closed defect (fixed)
Set_PythonType objects are not picklable
Reported by:  stumpc5  Owned by:  was 

Priority:  major  Milestone:  sage6.5 
Component:  pickling  Keywords:  
Cc:  sagecombinat  Merged in:  
Authors:  Vincent Delecroix, Jeroen Demeyer  Reviewers:  Jeroen Demeyer, Vincent Delecroix 
Report Upstream:  N/A  Work issues:  
Branch:  f28f02a (Commits, GitHub, GitLab)  Commit:  f28f02aeb6008daa88a267dc31d31963664c17dc 
Dependencies:  Stopgaps: 
Description (last modified by )
sage: S = sage.structure.parent.Set_PythonType(tuple) sage: dumps(S)  Traceback (most recent call last): File "<ipython console>", line 1, in <module> File "sage_object.pyx", line 842, in sage.structure.sage_object.dumps (sage/structure/sage_object.c:8274) File "sage_object.pyx", line 217, in sage.structure.sage_object.SageObject.dumps (sage/structure/sage_object.c:2183) PicklingError: Can't pickle <type 'function'>: attribute lookup __builtin__.function failed
Attachments (1)
Change History (33)
comment:1 Changed 10 years ago by
 Cc sagecombinat added
 Component changed from combinatorics to pickling
 Description modified (diff)
 Owner changed from sagecombinat to was
 Summary changed from Problem with pickle in CombinatorialFreeModule to Set_PythonType objects are not picklable
comment:2 Changed 8 years ago by
comment:3 Changed 8 years ago by
 Status changed from new to needs_review
Changed 8 years ago by
comment:4 Changed 8 years ago by
I undo a change which produces error (because of some metaclass buisness). I also add a method is_finite in relation with the cardinality one.
apply only: trac_10962pickle_set_pythontype.2.patch
comment:5 Changed 8 years ago by
Hi,
I added a relatively fast check at the very begining of the init method that verifies that the argument is a type or not (using the function PyType_Check
from the CAPI. It is the cause of many errors (see the log of the patchbot) and I do not know whether it is a good thing to check...
See the following sagedevel thread.
comment:6 Changed 6 years ago by
 Branch set to u/vdelecroix/10962
 Commit set to 18b968d45aa8649698eae4e5e70a88f9964e0e0d
New commits:
18b968d  trac #10962: make Set_PythonType pickable

comment:7 Changed 6 years ago by
 Status changed from needs_review to needs_work
This is wrong, it should be EXAMPLES:
:
EXAMPLES:: Let us create a parent in the category of rings::
comment:8 Changed 6 years ago by
 Commit changed from 18b968d45aa8649698eae4e5e70a88f9964e0e0d to 97f39849ff27767c6a99608d3a29db9b5738ae01
Branch pushed to git repo; I updated commit sha1. New commits:
97f3984  trac #10962: fix documentation

comment:9 Changed 6 years ago by
 Status changed from needs_work to needs_review
comment:10 Changed 6 years ago by
I got a strange error from the pickle jar. Could anyone perform the test?
sage t long structure/sage_object.pyx ********************************************************************** File "structure/sage_object.pyx", line 1431, in sage.structure.sage_object.unpickle_all Failed example: sage.structure.sage_object.unpickle_all() # (4s on sage.math, 2011) Expected: doctest:... DeprecationWarning: ... See http://trac.sagemath.org/... for details. Successfully unpickled ... objects. Failed to unpickle 0 objects. Got: doctest:1: DeprecationWarning: OrderedAlphabet is deprecated; use Alphabet instead. See http://trac.sagemath.org/8920 for details. doctest:850: DeprecationWarning: This class is replaced by Matrix_modn_dense_float/Matrix_modn_dense_double. See http://trac.sagemath.org/4260 for details. * unpickle failure: load('/home/vincent/.sage/temp/mangouste/23352/dir_D94KcT//pickle_jar/_class__sage_groups_matrix_gps_matrix_group_element_MatrixGroupElement__.sobj') doctest:1: DeprecationWarning: gen_py.pari is deprecated, use sage.libs.pari.all.pari instead See http://trac.sagemath.org/17451 for details. doctest:632: DeprecationWarning: The "pari_mod" finite field implementation is deprecated See http://trac.sagemath.org/17297 for details. Failed: _class__sage_groups_matrix_gps_matrix_group_element_MatrixGroupElement__.sobj  ** This error is probably due to an old pickle failing to unpickle. ** See sage.structure.sage_object.register_unpickle_override for ** how to override the default unpickling methods for (old) pickles. ** NOTE: pickles should never be removed from the pickle_jar!  Successfully unpickled 585 objects. Failed to unpickle 1 objects. ********************************************************************** File "structure/sage_object.pyx", line 1439, in sage.structure.sage_object.unpickle_all Failed example: sage.structure.sage_object.unpickle_all() Expected: Successfully unpickled ... objects. Failed to unpickle 0 objects. Got: * unpickle failure: load('/home/vincent/.sage/temp/mangouste/23352/dir_VbqHGc//pickle_jar/_class__sage_groups_matrix_gps_matrix_group_element_MatrixGroupElement__.sobj') Failed: _class__sage_groups_matrix_gps_matrix_group_element_MatrixGroupElement__.sobj  ** This error is probably due to an old pickle failing to unpickle. ** See sage.structure.sage_object.register_unpickle_override for ** how to override the default unpickling methods for (old) pickles. ** NOTE: pickles should never be removed from the pickle_jar!  Successfully unpickled 585 objects. Failed to unpickle 1 objects. **********************************************************************
comment:11 Changed 6 years ago by
 Branch changed from u/vdelecroix/10962 to u/jdemeyer/ticket/10962
 Modified changed from 12/19/14 17:46:14 to 12/19/14 17:46:14
comment:12 Changed 6 years ago by
 Commit changed from 97f39849ff27767c6a99608d3a29db9b5738ae01 to 2908e5797bb558895695abd5443ad718cc14d5fb
I confirm the unpickling error, it is due to
TypeError Traceback (most recent call last) <ipythoninput3d62ad2672fb4> in <module>() > 1 load('/home/jdemeyer/.sage/temp/tamiyo/6215/dir_LJVs2X//pickle_jar/_class__sage_groups_matrix_gps_matrix_group_element_MatrixGroupElement__.sobj') /usr/local/src/sageconfig/src/sage/structure/sage_object.pyx in sage.structure.sage_object.load (build/cythonized/sage/structure/sage_object.c:11233)() 945 946 ## Load file by absolute filename > 947 X = loads(open(filename).read(), compress=compress) 948 try: 949 X._default_filename = os.path.abspath(filename) /usr/local/src/sageconfig/src/sage/structure/sage_object.pyx in sage.structure.sage_object.loads (build/cythonized/sage/structure/sage_object.c:13238)() 1319 unpickler.find_global = unpickle_global 1320 > 1321 return unpickler.load() 1322 1323 /usr/local/src/sageconfig/local/lib/python2.7/sitepackages/sage/groups/matrix_gps/pickling_overrides.pyc in __setstate__(self, state) 36 degree = state['_MatrixGroup_gap__n'] 37 from sage.libs.all import libgap > 38 libgap_group = libgap.Group(libgap(matrix_gens)) 39 self.__init__(degree, ring, libgap_group) 40 /usr/local/src/sageconfig/src/sage/structure/parent.pyx in sage.structure.parent.Parent.__call__ (build/cythonized/sage/structure/parent.c:9585)() 1102 mor = <map.Map> self._convert_from_hash.get(R) 1103 except KeyError: > 1104 mor = <map.Map> self._internal_convert_map_from(R) 1105 1106 if mor is not None: /usr/local/src/sageconfig/src/sage/structure/parent.pyx in sage.structure.parent.Parent._internal_convert_map_from (build/cythonized/sage/structure/parent.c:19460)() 2581 return self._convert_from_hash.get(S) 2582 except KeyError: > 2583 mor = self.discover_convert_map_from(S) 2584 # Before trac #14711, the morphism has been 2585 # put both into _convert_from_list and into /usr/local/src/sageconfig/src/sage/structure/parent.pyx in sage.structure.parent.Parent.discover_convert_map_from (build/cythonized/sage/structure/parent.c:20064)() 2624 from sage.structure.coerce_maps import ListMorphism 2625 if isinstance(S, Sequences): > 2626 return ListMorphism(S, self.convert_map_from(list)) 2627 2628 mor = self._generic_convert_map(S) /usr/local/src/sageconfig/src/sage/structure/coerce_maps.pyx in sage.structure.coerce_maps.ListMorphism.__init__ (build/cythonized/sage/structure/coerce_maps.c:8911)() 529 def __init__(self, domain, Map real_morphism): 530 if not PY_TYPE_CHECK(domain, Parent): > 531 domain = Set_PythonType(domain) 532 Map.__init__(self, domain, real_morphism.codomain()) 533 self._coerce_cost = real_morphism._coerce_cost + 3 /usr/local/src/sageconfig/src/sage/structure/parent.pyx in sage.structure.parent.Set_PythonType (build/cythonized/sage/structure/parent.c:24290)() 2990 cdef _type_set_cache = {} 2991 > 2992 cpdef Parent Set_PythonType(theType): 2993 """ 2994 Return the (unique) Parent that represents the set of Python objects /usr/local/src/sageconfig/src/sage/structure/parent.pyx in sage.structure.parent.Set_PythonType (build/cythonized/sage/structure/parent.c:24199)() 3021 return _type_set_cache[theType] 3022 except KeyError: > 3023 _type_set_cache[theType] = theSet = Set_PythonType_class(theType) 3024 return theSet 3025 /usr/local/src/sageconfig/src/sage/structure/parent.pyx in sage.structure.parent.Set_PythonType_class.__init__ (build/cythonized/sage/structure/parent.c:24417)() 3055 """ 3056 if not isinstance(theType, type): > 3057 raise TypeError("must be intialized with a type, not %r" % theType) 3058 Set_generic.__init__(self, element_constructor=theType, category=Sets()) 3059 self._type = theType TypeError: must be intialized with a type, not Category of sequences in Full MatrixSpace of 2 by 2 dense matrices over Finite Field of size 3
New commits:
2908e57  Remove useless C/API calls

comment:13 followup: ↓ 15 Changed 6 years ago by
Question: would it be valid to use Set_PythonType()
on a Category
instead of a type
?
comment:14 Changed 6 years ago by
 Milestone set to sage6.5
 Status changed from needs_review to needs_work
comment:15 in reply to: ↑ 13 ; followup: ↓ 22 Changed 6 years ago by
Replying to jdemeyer:
Question: would it be valid to use
Set_PythonType()
on aCategory
instead of atype
?
No. By the way I do not understand why Posets
and Sequences
are categories and not Parent
...
comment:16 Changed 6 years ago by
 Branch changed from u/jdemeyer/ticket/10962 to u/vdelecroix/10962
 Commit changed from 2908e5797bb558895695abd5443ad718cc14d5fb to da81792004986e23a3dee4c92b0cc229b64741f2
 Status changed from needs_work to needs_review
New commits:
da81792  trac #10962: fix unpickle (Sequences is not a parent)

comment:17 Changed 6 years ago by
 Branch changed from u/vdelecroix/10962 to u/jdemeyer/ticket/10962
 Modified changed from 12/20/14 07:26:22 to 12/20/14 07:26:22
comment:18 Changed 6 years ago by
 Commit changed from da81792004986e23a3dee4c92b0cc229b64741f2 to 4b5c3732a910b99b8f43b5b4eb0da13c8f1c7f27
 Reviewers set to Jeroen Demeyer
I added one extra commit to simplify the code and remove a compiler warning for parent.pyx
(which is not your fault, but which should be fixed anyway).
New commits:
4b5c373  Remove useless C/API calls; improve error message

comment:19 Changed 6 years ago by
 Status changed from needs_review to needs_work
sage t long src/sage/symbolic/maxima_wrapper.py ********************************************************************** File "src/sage/symbolic/maxima_wrapper.py", line 113, in sage.symbolic.maxima_wrapper.MaximaWrapper._symbolic_ Failed example: SR(u) is t # indirect doctest Exception raised: Traceback (most recent call last): File "/usr/local/src/sageconfig/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 488, in _run self.compile_and_execute(example, compiler, test.globs) File "/usr/local/src/sageconfig/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 850, in compile_and_execute exec(compiled, globs) File "<doctest sage.symbolic.maxima_wrapper.MaximaWrapper._symbolic_[2]>", line 1, in <module> SR(u) is t # indirect doctest File "sage/structure/parent.pyx", line 1104, in sage.structure.parent.Parent.__call__ (build/cythonized/sage/structure/parent.c:9585) mor = <map.Map> self._internal_convert_map_from(R) File "sage/structure/parent.pyx", line 2583, in sage.structure.parent.Parent._internal_convert_map_from (build/cythonized/sage/structure/parent.c:19460) mor = self.discover_convert_map_from(S) File "sage/structure/parent.pyx", line 2632, in sage.structure.parent.Parent.discover_convert_map_from (build/cythonized/sage/structure/parent.c:20119) mor = self._generic_convert_map(S) File "sage/structure/parent_old.pyx", line 534, in sage.structure.parent_old.Parent._generic_convert_map (build/cythonized/sage/structure/parent_old.c:8211) return parent.Parent._generic_convert_map(self, S) File "sage/structure/parent.pyx", line 2142, in sage.structure.parent.Parent._generic_convert_map (build/cythonized/sage/structure/parent.c:16530) return coerce_maps.DefaultConvertMap_unique(S, self) File "sage/structure/coerce_maps.pyx", line 39, in sage.structure.coerce_maps.DefaultConvertMap.__init__ (build/cythonized/sage/structure/coerce_maps.c:2712) domain = Set_PythonType(domain) File "sage/structure/parent.pyx", line 2996, in sage.structure.parent.Set_PythonType (build/cythonized/sage/structure/parent.c:24328) cpdef Parent Set_PythonType(theType): File "sage/structure/parent.pyx", line 3027, in sage.structure.parent.Set_PythonType (build/cythonized/sage/structure/parent.c:24237) _type_set_cache[theType] = theSet = Set_PythonType_class(theType) File "sage/structure/parent.pyx", line 3061, in sage.structure.parent.Set_PythonType_class.__init__ (build/cythonized/sage/structure/parent.c:24455) raise TypeError("must be intialized with a type, not %r" % theType) TypeError: must be intialized with a type, not parent(log(sqrt(2) + 1) + log(sqrt(2)  1)) **********************************************************************
comment:20 Changed 6 years ago by
 Commit changed from 4b5c3732a910b99b8f43b5b4eb0da13c8f1c7f27 to edf526e2a9b181c058620e1b42b1b378c37d99a7
Branch pushed to git repo; I updated commit sha1. New commits:
edf526e  Define a parent() method for MaximaWrapper objects

comment:21 Changed 6 years ago by
 Status changed from needs_work to needs_review
comment:22 in reply to: ↑ 15 ; followup: ↓ 24 Changed 6 years ago by
Replying to vdelecroix:
Replying to jdemeyer:
Question: would it be valid to use
Set_PythonType()
on aCategory
instead of atype
?No. By the way I do not understand why
Posets
andSequences
are categories and notParent
...
The category corresponding to Parent
is Sets
. Now, why is there both Parent
and Sets
? The latter is an abstract class, while the former is a concrete implementation (and currently the only concrete base implementation; but we could imagine having others). Of course, a lot of the current features of Parent
really should be moved up in Sets
or in other categories. Some just need to be taken care of (handling some backward compatibility issues). Some are more intrinsically difficult
because of the lack of multiple inheritance in Cython; that's analog to the Ring
versus Rings
situation.
comment:23 followup: ↓ 25 Changed 6 years ago by
This finally passes all doctests. We just need somebody to review my last two commits:
comment:24 in reply to: ↑ 22 ; followup: ↓ 26 Changed 6 years ago by
Replying to nthiery:
Replying to vdelecroix:
Replying to jdemeyer:
Question: would it be valid to use
Set_PythonType()
on aCategory
instead of atype
?No. By the way I do not understand why
Posets
andSequences
are categories and notParent
...The category corresponding to
Parent
isSets
. Now, why is there bothParent
andSets
? The latter is an abstract class, while the former is a concrete implementation (and currently the only concrete base implementation; but we could imagine having others). Of course, a lot of the current features ofParent
really should be moved up inSets
or in other categories. Some just need to be taken care of (handling some backward compatibility issues). Some are more intrinsically difficult because of the lack of multiple inheritance in Cython; that's analog to theRing
versusRings
situation.
Thanks Nicolas. But I do not understand why it is an answer to my question: "why Posets and Sequences are categories and not Parent?" As far as I understand, to declare something to have a category it needs to inherit from Parent (via Parent.__init__(category=XYZ)
). So I do not see how to avoid Parent
by "having others".
The issue of Ring
versus Rings
has nothing to do with my question, or I missed something.
Vincent
comment:25 in reply to: ↑ 23 ; followups: ↓ 27 ↓ 29 Changed 6 years ago by
 Status changed from needs_review to needs_info
Replying to jdemeyer:
This finally passes all doctests. We just need somebody to review my last two commits:
I am fine with the first one. But the result of .parent()
should be a parent isn't it? Why not
def parent(self): return Set_PythonType(type(self))
On the other hand, the MaximaWrapper
does not inherit from Element
which is perfectly fine. I am worried that we need this method. The bug for me is the following behavior
sage: parent(sin(1).maxima_methods()) parent(sin(1))
which should just behave like
sage: parent(int(1)) <type 'int'>
(which is also strange but perfectly fine if uniform).
The reason is that there is a too permissive __getattr__
in MaximaWrapper
which never raises AttributeError
. I would really prefer that:
 either we move your solution at the level of
SageObject
 or we hack
__getattr__
ofMaximaWrapper
Vincent
comment:26 in reply to: ↑ 24 ; followup: ↓ 30 Changed 6 years ago by
Replying to vdelecroix:
Replying to nthiery:
Replying to vdelecroix:
Replying to jdemeyer:
Question: would it be valid to use
Set_PythonType()
on aCategory
instead of atype
?No. By the way I do not understand why
Posets
andSequences
are categories and notParent
...The category corresponding to
Parent
isSets
. Now, why is there bothParent
andSets
? The latter is an abstract class, while the former is a concrete implementation (and currently the only concrete base implementation; but we could imagine having others). Of course, a lot of the current features ofParent
really should be moved up inSets
or in other categories. Some just need to be taken care of (handling some backward compatibility issues). Some are more intrinsically difficult because of the lack of multiple inheritance in Cython; that's analog to theRing
versusRings
situation.Thanks Nicolas. But I do not understand why it is an answer to my question: "why Posets and Sequences are categories and not Parent?" As far as I understand, to declare something to have a category it needs to inherit from Parent (via
Parent.__init__(category=XYZ)
). So I do not see how to avoidParent
by "having others". The issue ofRing
versusRings
has nothing to do with my question, or I missed something.
Hmm I am not sure I understand your question then. Can you rephrase it?
Just a quick note for now: to have a category, you need to be a CategoryObject? (surprise surprise ...).
comment:27 in reply to: ↑ 25 Changed 6 years ago by
Replying to vdelecroix:
But the result of
.parent()
should be a parent isn't it?
It doesn't have to be. For things which don't have a parent, it's accepted to return a type
instead. After all, the global parent(x)
function returns type(x)
if x has no parent.
comment:28 Changed 6 years ago by
 Commit changed from edf526e2a9b181c058620e1b42b1b378c37d99a7 to f28f02aeb6008daa88a267dc31d31963664c17dc
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
f28f02a  Define parent() to be type() for SageObject

comment:29 in reply to: ↑ 25 Changed 6 years ago by
 Status changed from needs_info to needs_review
Replying to vdelecroix:
 either we move your solution at the level of
SageObject
Done! This commit replaces the last one.
comment:30 in reply to: ↑ 26 Changed 6 years ago by
Replying to nthiery:
Hmm I am not sure I understand your question then. Can you rephrase it?
Why Posets
and Sequences
do not inherit from CategoryObject
?
Vincent
comment:31 Changed 6 years ago by
 Reviewers changed from Jeroen Demeyer to Jeroen Demeyer, Vincent Delecroix
 Status changed from needs_review to positive_review
All right. Thanks.
comment:32 Changed 6 years ago by
 Branch changed from u/jdemeyer/ticket/10962 to f28f02aeb6008daa88a267dc31d31963664c17dc
 Resolution set to fixed
 Status changed from positive_review to closed
I implement a
__reduce__
method for Set_PythonType and now it works...Best, Vincent