Opened 5 years ago

Closed 5 years ago

#24431 closed enhancement (fixed)

Fix coercions and pushout for Laurent series

Reported by: Vincent Delecroix Owned by:
Priority: major Milestone: sage-8.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:

Status badges

Description (last modified by Travis Scrimshaw)

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 Vincent Delecroix

Branch: u/vdelecroix/24431
Commit: eb2bba461ab15f7cbb21733d42797af8d17f3464
Component: PLEASE CHANGEalgebra
Status: newneeds_review

New commits:

f779b1024420: clean laurent series
eb2bba424431: pushout for Laurent series

comment:2 Changed 5 years ago by git

Commit: eb2bba461ab15f7cbb21733d42797af8d17f346427f324eddb758edbeaeee37fde25dfd74e94d888

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

1adb84c24420: clean laurent series
27f324e24431: pushout for Laurent series

comment:3 Changed 5 years ago by git

Commit: 27f324eddb758edbeaeee37fde25dfd74e94d88800deb276b62aaadc5d1a77a1965821e3efb2def6

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

cdf836a24420: clean laurent series
00deb2724431: pushout for Laurent series

comment:4 Changed 5 years ago by git

Commit: 00deb276b62aaadc5d1a77a1965821e3efb2def6c905ee96cc6f80e679cc55ddaf9e02380e18672b

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

f95f61e24413: make polynomial rings know that they are infinite
c905ee924413: base ring = 0

comment:5 Changed 5 years ago by git

Commit: c905ee96cc6f80e679cc55ddaf9e02380e18672b00deb276b62aaadc5d1a77a1965821e3efb2def6

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

cdf836a24420: clean laurent series
00deb2724431: pushout for Laurent series

comment:6 Changed 5 years ago by Vincent Delecroix

Description: modified (diff)
Status: needs_reviewneeds_work
Summary: Pushout for Laurent seriesFix coercions and pushout for Laurent series

comment:7 Changed 5 years ago by git

Commit: 00deb276b62aaadc5d1a77a1965821e3efb2def61b1bbbc062f24af9adecd11ddab5a12bf8812636

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

1b1bbbc24431: Laurent series pushout and coercions

comment:8 Changed 5 years ago by Vincent Delecroix

Description: modified (diff)
Status: needs_workneeds_review

comment:9 Changed 5 years ago by git

Commit: 1b1bbbc062f24af9adecd11ddab5a12bf8812636b3f11d9d244e72085a96d64e1286aa43f70df44f

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

34769e124420: clean laurent series
b3f11d924431: Laurent series pushout and coercions

comment:10 Changed 5 years ago by Vincent Delecroix

rebased on 8.2.beta2

comment:11 Changed 5 years ago by Travis Scrimshaw

LGTM except I don't understand the non-import changes in schemes/hyperelliptic_curves/hyperelliptic_generic.py. (Also, is LaurentPolynomialRing is not used in that file.)

comment:12 in reply to:  11 Changed 5 years ago by Vincent Delecroix

Replying to tscrim:

LGTM except I don't understand the non-import changes in schemes/hyperelliptic_curves/hyperelliptic_generic.py. (Also, is LaurentPolynomialRing 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 Travis Scrimshaw

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 git

Commit: b3f11d9d244e72085a96d64e1286aa43f70df44fde63301a78c5746c7d3384a19bac1a4e7564741c

Branch pushed to git repo; I updated commit sha1. New commits:

de6330124431: unneeded '\' and more tests

comment:15 Changed 5 years ago by Vincent Delecroix

I did not find the import you were talking about in 13.

comment:16 Changed 5 years ago by Travis Scrimshaw

  • 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 
    3333#                  http://www.gnu.org/licenses/
    3434#*****************************************************************************
    3535
    36 from sage.rings.all import PolynomialRing, RR, PowerSeriesRing, LaurentSeriesRing, O
     36from sage.rings.polynomial.all import PolynomialRing, LaurentPolynomialRing
     37from sage.rings.big_oh import O
     38from sage.rings.power_series_ring import PowerSeriesRing
     39from sage.rings.laurent_series_ring import LaurentSeriesRing
     40from sage.rings.real_mpfr import RR
    3741from sage.functions.all import log
    3842from sage.structure.category_object import normalize_names
    3943

comment:17 Changed 5 years ago by git

Commit: de63301a78c5746c7d3384a19bac1a4e7564741ccd4b485c31009875acf25e8082477b2da7288ee5

Branch pushed to git repo; I updated commit sha1. New commits:

cd4b48524431: remove unused import

comment:18 Changed 5 years ago by Marc Mezzarobba

Status: needs_reviewneeds_work

Looks like there is a test failure since 8.2.beta3.

comment:19 Changed 5 years ago by Travis Scrimshaw

It is trivial due to the output of the functor. Once fixed, you can set a positive review on my behalf.

Last edited 5 years ago by Travis Scrimshaw (previous) (diff)

comment:20 Changed 5 years ago by git

Commit: cd4b485c31009875acf25e8082477b2da7288ee58abc33b3dd7001c7022e9c114563251d7e668304

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

8abc33b24431: Laurent series pushout and coercions

comment:21 Changed 5 years ago by Vincent Delecroix

Status: needs_workneeds_review

Rebased on beta4. Waiting for patchbots as an extra check.

comment:22 Changed 5 years ago by Vincent Delecroix

Status: needs_reviewpositive_review

comment:23 Changed 5 years ago by Volker Braun

Branch: u/vdelecroix/244318abc33b3dd7001c7022e9c114563251d7e668304
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.