Opened 4 years ago
Closed 4 years ago
#26354 closed defect (fixed)
Pickling morphisms is broken
Reported by: | caruso | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-8.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 4 years ago by
- Branch set to u/roed/pickling_morphisms_is_broken
comment:2 Changed 4 years ago by
- Commit set to 94f6985f60c352044b4d99bf8fc15818c17c7281
- Status changed from new to needs_review
comment:3 Changed 4 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 4 years ago by
- Branch changed from u/roed/pickling_morphisms_is_broken to u/caruso/pickling_morphisms_is_broken
comment:5 Changed 4 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 4 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 4 years ago by
- Reviewers set to Travis Scrimshaw
- Status changed from needs_review to positive_review
Yep, LGTM. Thanks.
comment:8 Changed 4 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