Opened 5 years ago
Closed 5 years ago
#24431 closed enhancement (fixed)
Fix coercions and pushout for Laurent series
Reported by:  vdelecroix  Owned by:  

Priority:  major  Milestone:  sage8.2 
Component:  algebra  Keywords:  
Cc:  Merged in:  
Authors:  Vincent Delecroix  Reviewers:  Travis Scrimshaw 
Report Upstream:  N/A  Work issues:  
Branch:  8abc33b (Commits, GitHub, GitLab)  Commit:  8abc33b3dd7001c7022e9c114563251d7e668304 
Dependencies:  #24420  Stopgaps: 
Description (last modified by )
There should be an automatic extension of scalars in the following situation
sage: R.<x> = LaurentSeriesRing(QQ) sage: QQbar.gen() * x Traceback (most recent call last): ... TypeError: unsupported operand parent(s) for *: 'Algebraic Field' and 'Laurent Series Ring in x over Rational Field'
To be compared with
sage: R.<x> = PowerSeriesRing(QQ) sage: QQbar.gen() * x I*x sage: parent(_) Power Series Ring in x over Algebraic Field
Currently, these pushouts are allowed:
sage: x = LaurentSeriesRing(QQ, 'x').gen() sage: y = PolynomialRing(QQ, 'y').gen() sage: x * y x*y sage: parent(x * y) Univariate Polynomial Ring in y over Laurent Series Ring in x over Rational Field
This is inconsistent with PowerSeriesRing
.
By fixing these problems, we had to adapt some code in sage/schemes/hyperelliptic_curves/hyperelliptic_generic.py
that was using the aforementioned pushout.
Change History (23)
comment:1 Changed 5 years ago by
Branch:  → u/vdelecroix/24431 

Commit:  → eb2bba461ab15f7cbb21733d42797af8d17f3464 
Component:  PLEASE CHANGE → algebra 
Status:  new → needs_review 
comment:2 Changed 5 years ago by
Commit:  eb2bba461ab15f7cbb21733d42797af8d17f3464 → 27f324eddb758edbeaeee37fde25dfd74e94d888 

comment:3 Changed 5 years ago by
Commit:  27f324eddb758edbeaeee37fde25dfd74e94d888 → 00deb276b62aaadc5d1a77a1965821e3efb2def6 

comment:4 Changed 5 years ago by
Commit:  00deb276b62aaadc5d1a77a1965821e3efb2def6 → c905ee96cc6f80e679cc55ddaf9e02380e18672b 

comment:5 Changed 5 years ago by
Commit:  c905ee96cc6f80e679cc55ddaf9e02380e18672b → 00deb276b62aaadc5d1a77a1965821e3efb2def6 

comment:6 Changed 5 years ago by
Description:  modified (diff) 

Status:  needs_review → needs_work 
Summary:  Pushout for Laurent series → Fix coercions and pushout for Laurent series 
comment:7 Changed 5 years ago by
Commit:  00deb276b62aaadc5d1a77a1965821e3efb2def6 → 1b1bbbc062f24af9adecd11ddab5a12bf8812636 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
1b1bbbc  24431: Laurent series pushout and coercions

comment:8 Changed 5 years ago by
Description:  modified (diff) 

Status:  needs_work → needs_review 
comment:9 Changed 5 years ago by
Commit:  1b1bbbc062f24af9adecd11ddab5a12bf8812636 → b3f11d9d244e72085a96d64e1286aa43f70df44f 

comment:11 followup: 12 Changed 5 years ago by
LGTM except I don't understand the nonimport changes in schemes/hyperelliptic_curves/hyperelliptic_generic.py
. (Also, is LaurentPolynomialRing
is not used in that file.)
comment:12 Changed 5 years ago by
Replying to tscrim:
LGTM except I don't understand the nonimport changes in
schemes/hyperelliptic_curves/hyperelliptic_generic.py
. (Also, isLaurentPolynomialRing
is not used in that file.)
I believe you refer to this line
 L = PolynomialRing(self.base_ring(),'x') + L = PolynomialRing(K,'x')
This is the third point in the ticket description.
comment:13 Changed 5 years ago by
Description:  modified (diff) 

Reviewers:  → Travis Scrimshaw 
I understand now the reason why that change is needed. Thanks. (Although I don't quite understand why that pushout is no longer allowed, but that's okay.)
Just remove the unneeded import of LaurentPolynomialRing
and the \
's here
@@ 410,7 +565,11 @@ class LaurentSeriesRing_generic(UniqueRepresentation, ring.CommutativeRing): from sage.rings.polynomial.polynomial_ring import is_PolynomialRing from sage.rings.power_series_ring import is_PowerSeriesRing  if ((is_LaurentSeriesRing(P) or is_PowerSeriesRing(P) or is_PolynomialRing(P)) + from sage.rings.polynomial.laurent_polynomial_ring import is_LaurentPolynomialRing + if ((is_LaurentSeriesRing(P) or \ + is_LaurentPolynomialRing(P) or \ + is_PowerSeriesRing(P) or \ + is_PolynomialRing(P)) and P.variable_name() == self.variable_name() and A.has_coerce_map_from(P.base_ring())): return True
and you can set a positive review on my behalf.
comment:14 Changed 5 years ago by
Commit:  b3f11d9d244e72085a96d64e1286aa43f70df44f → de63301a78c5746c7d3384a19bac1a4e7564741c 

Branch pushed to git repo; I updated commit sha1. New commits:
de63301  24431: unneeded '\' and more tests

comment:16 Changed 5 years ago by

src/sage/schemes/hyperelliptic_curves/hyperelliptic_generic.py
diff git a/src/sage/schemes/hyperelliptic_curves/hyperelliptic_generic.py b/src/sage/schemes/hyperelliptic_curves/hyperelliptic_generic.py index cdca282..9e9897a 100644
a b from __future__ import absolute_import 33 33 # http://www.gnu.org/licenses/ 34 34 #***************************************************************************** 35 35 36 from sage.rings.all import PolynomialRing, RR, PowerSeriesRing, LaurentSeriesRing, O 36 from sage.rings.polynomial.all import PolynomialRing, LaurentPolynomialRing 37 from sage.rings.big_oh import O 38 from sage.rings.power_series_ring import PowerSeriesRing 39 from sage.rings.laurent_series_ring import LaurentSeriesRing 40 from sage.rings.real_mpfr import RR 37 41 from sage.functions.all import log 38 42 from sage.structure.category_object import normalize_names 39 43
comment:17 Changed 5 years ago by
Commit:  de63301a78c5746c7d3384a19bac1a4e7564741c → cd4b485c31009875acf25e8082477b2da7288ee5 

Branch pushed to git repo; I updated commit sha1. New commits:
cd4b485  24431: remove unused import

comment:18 Changed 5 years ago by
Status:  needs_review → needs_work 

Looks like there is a test failure since 8.2.beta3.
comment:19 Changed 5 years ago by
It is trivial due to the output of the function. Once fixed, you can set a positive review on my behalf.
comment:20 Changed 5 years ago by
Commit:  cd4b485c31009875acf25e8082477b2da7288ee5 → 8abc33b3dd7001c7022e9c114563251d7e668304 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
8abc33b  24431: Laurent series pushout and coercions

comment:21 Changed 5 years ago by
Status:  needs_work → needs_review 

Rebased on beta4. Waiting for patchbots as an extra check.
comment:22 Changed 5 years ago by
Status:  needs_review → positive_review 

comment:23 Changed 5 years ago by
Branch:  u/vdelecroix/24431 → 8abc33b3dd7001c7022e9c114563251d7e668304 

Resolution:  → fixed 
Status:  positive_review → closed 
New commits:
24420: clean laurent series
24431: pushout for Laurent series