Opened 5 years ago

Closed 5 years ago

#24420 closed defect (fixed)

Laurent power series fail unique representation

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: bcf577d (Commits, GitHub, GitLab) Commit: bcf577dc4c384de604bb60577bdc08f53e7fba18
Dependencies: Stopgaps:

Status badges

Description (last modified by Vincent Delecroix)

sage: L.<q> = LaurentSeriesRing(QQ)
sage: LaurentSeriesRing(QQ, 'q') is L
False

to be compared with

sage: R.<q> = PolynomialRing(QQ)
sage: PolynomialRing(QQ, 'q') is R
True

We also fix

sage: LaurentSeriesRing(Zmod(4), 'x').category()
Category of fields

We also remove the classes LaurentSeriesRing_generic, LaurentSeriesRing_domain, LaurentSeriesRing_field in favor of a unique LaurentSeriesRing.

Change History (18)

comment:1 Changed 5 years ago by Vincent Delecroix

Description: modified (diff)

comment:2 Changed 5 years ago by Vincent Delecroix

Description: modified (diff)

comment:3 Changed 5 years ago by Vincent Delecroix

Authors: Vincent Delecroix
Description: modified (diff)

comment:4 Changed 5 years ago by Vincent Delecroix

Branch: u/vdelecroix/24420
Commit: f779b10d261e9ff4f782ac53e978af5b62b83f3a
Description: modified (diff)
Status: newneeds_review

New commits:

f779b1024420: clean laurent series

comment:5 Changed 5 years ago by git

Commit: f779b10d261e9ff4f782ac53e978af5b62b83f3a1adb84c9287ef49d822548fb94f28180a2b8753c

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

1adb84c24420: clean laurent series

comment:6 Changed 5 years ago by git

Commit: 1adb84c9287ef49d822548fb94f28180a2b8753ccdf836ac796a696568ddbae346e58bed6327405a

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

cdf836a24420: clean laurent series

comment:7 Changed 5 years ago by git

Commit: cdf836ac796a696568ddbae346e58bed6327405a34769e112def9b211234bed663889486e55b95a5

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

34769e124420: clean laurent series

comment:8 Changed 5 years ago by Vincent Delecroix

rebased on 8.2.beta2

comment:9 Changed 5 years ago by Travis Scrimshaw

Reviewers: Travis Scrimshaw

Since you are going to do the category imports in the __init__ (which I'm not 100% in agreement with), I would make it in as small as scope as possible:

         from sage.categories.rings import Rings
         from sage.categories.integral_domains import IntegralDomains
         from sage.categories.fields import Fields
-        from sage.categories.complete_discrete_valuation import (
-                CompleteDiscreteValuationRings, CompleteDiscreteValuationFields)
 
         base_ring = power_series.base_ring()
         if base_ring in Fields():
+            from sage.categories.complete_discrete_valuation import CompleteDiscreteValuationFields
             category = CompleteDiscreteValuationFields()
         elif base_ring in IntegralDomains():
             category = IntegralDomains()
         elif base_ring in Rings().Commutative():
             category = Rings().Commutative()
         else:
             raise ValueError('unrecognized base ring')

Also, don't we want to use names=power_series.variable_names() in case we (eventually support) have multiple variable names. Otherwise LGTM.

comment:10 in reply to:  9 ; Changed 5 years ago by Vincent Delecroix

Replying to tscrim:

Since you are going to do the category imports in the __init__ (which I'm not 100% in agreement with)

Why? I am happy to move them at the module scope if it is a better practice.

comment:11 in reply to:  10 Changed 5 years ago by Travis Scrimshaw

Replying to vdelecroix:

Replying to tscrim:

Since you are going to do the category imports in the __init__ (which I'm not 100% in agreement with)

Why? I am happy to move them at the module scope if it is a better practice.

It slows down the construction of the ring.

comment:12 Changed 5 years ago by git

Commit: 34769e112def9b211234bed663889486e55b95a5fd34a9565beb04c9cdab5ab8c97d64db3892c474

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

fd34a9524420: move imports / name -> names

comment:13 Changed 5 years ago by Vincent Delecroix

done

comment:14 Changed 5 years ago by Travis Scrimshaw

Thanks. One last little thing: you don't use CompleteDiscreteValuationRings.

comment:15 Changed 5 years ago by git

Commit: fd34a9565beb04c9cdab5ab8c97d64db3892c474bcf577dc4c384de604bb60577bdc08f53e7fba18

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

bcf577d24420: move imports / name -> names

comment:16 Changed 5 years ago by Vincent Delecroix

Indeed!

comment:17 Changed 5 years ago by Travis Scrimshaw

Status: needs_reviewpositive_review

Thanks.

comment:18 Changed 5 years ago by Volker Braun

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