Opened 3 years ago
Closed 3 years ago
#26354 closed defect (fixed)
Pickling morphisms is broken
Reported by:  caruso  Owned by:  

Priority:  major  Milestone:  sage8.4 
Component:  pickling  Keywords:  
Cc:  Merged in:  
Authors:  David Roe, Xavier Caruso  Reviewers:  Travis Scrimshaw 
Report Upstream:  N/A  Work issues:  
Branch:  1ea91f7 (Commits, GitHub, GitLab)  Commit:  1ea91f7b6254f8791b4f0b488e32c002af7e99a0 
Dependencies:  Stopgaps: 
Description
Here is an exemple:
sage: R.<x,y> = QQ[] sage: theta = R.hom([y,x]) sage: hash(theta) 1982612945128833994 sage: hash(loads(dumps(theta))) Traceback (most recent call last): ... ValueError: mutable sequences are unhashable
Change History (8)
comment:1 Changed 3 years ago by
 Branch set to u/roed/pickling_morphisms_is_broken
comment:2 Changed 3 years ago by
 Commit set to 94f6985f60c352044b4d99bf8fc15818c17c7281
 Status changed from new to needs_review
comment:3 Changed 3 years ago by
This is definitely a fix, but is it the correct fix? The PolynomialSequence
does not seem to pickle immutability like I think it should:
sage: theta.__reduce__()[1][3]['__im_gens'] [y, x] sage: S = theta.__reduce__()[1][3]['__im_gens'] sage: type(S) <class 'sage.rings.polynomial.multi_polynomial_sequence.PolynomialSequence_generic'> sage: S.is_immutable() True sage: loads(dumps(S)).is_immutable() False
Contrast:
sage: S.__reduce__() (<function PolynomialSequence at 0x7f62c7ef2c80>, (Multivariate Polynomial Ring in x, y over Rational Field, ((y, x),))) sage: Sequence([1,2,3,4]).__reduce__() (<function _reconstructor at 0x7f64198a5aa0>, (<class 'sage.structure.sequence.Sequence_generic'>, <type 'sage.structure.sage_object.SageObject'>, <sage.structure.sage_object.SageObject object at 0x7f62b02dc0b0>), {'_Sequence_generic__cr': False, '_Sequence_generic__cr_str': False, '_Sequence_generic__hash': None, '_Sequence_generic__universe': Integer Ring, '_is_immutable': False})
What are your thoughts about instead adjusting the pickling of PolynomialSequence
?
comment:4 Changed 3 years ago by
 Branch changed from u/roed/pickling_morphisms_is_broken to u/caruso/pickling_morphisms_is_broken
comment:5 Changed 3 years ago by
 Commit changed from 94f6985f60c352044b4d99bf8fc15818c17c7281 to e2931b6b109cd7fb492945c93e90be7cd68a30bc
I agree with you, Travis.
I pushed a fix for the pickling of PolynomialSequence
. I'm not sure I've done it correctly (in particular, this class redefines __getattr__
, I don't know why); please review :)
New commits:
dee878e  Revert "Set immutable in unpickling ring morphisms"

e2931b6  Fix pickling for PolynomialSequence_generic

comment:6 Changed 3 years ago by
 Commit changed from e2931b6b109cd7fb492945c93e90be7cd68a30bc to 1ea91f7b6254f8791b4f0b488e32c002af7e99a0
Branch pushed to git repo; I updated commit sha1. New commits:
1ea91f7  Add a doctest

comment:7 Changed 3 years ago by
 Reviewers set to Travis Scrimshaw
 Status changed from needs_review to positive_review
Yep, LGTM. Thanks.
comment:8 Changed 3 years ago by
 Branch changed from u/caruso/pickling_morphisms_is_broken to 1ea91f7b6254f8791b4f0b488e32c002af7e99a0
 Resolution set to fixed
 Status changed from positive_review to closed
Note: See
TracTickets for help on using
tickets.
New commits:
Set immutable in unpickling ring morphisms