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:  sage8.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: 
Description (last modified by )
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
Description:  modified (diff) 

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

comment:3 Changed 5 years ago by
Authors:  → Vincent Delecroix 

Description:  modified (diff) 
comment:4 Changed 5 years ago by
Branch:  → u/vdelecroix/24420 

Commit:  → f779b10d261e9ff4f782ac53e978af5b62b83f3a 
Description:  modified (diff) 
Status:  new → needs_review 
comment:5 Changed 5 years ago by
Commit:  f779b10d261e9ff4f782ac53e978af5b62b83f3a → 1adb84c9287ef49d822548fb94f28180a2b8753c 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
1adb84c  24420: clean laurent series

comment:6 Changed 5 years ago by
Commit:  1adb84c9287ef49d822548fb94f28180a2b8753c → cdf836ac796a696568ddbae346e58bed6327405a 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
cdf836a  24420: clean laurent series

comment:7 Changed 5 years ago by
Commit:  cdf836ac796a696568ddbae346e58bed6327405a → 34769e112def9b211234bed663889486e55b95a5 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
34769e1  24420: clean laurent series

comment:9 followup: 10 Changed 5 years ago by
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 followup: 11 Changed 5 years ago by
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 Changed 5 years ago by
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
Commit:  34769e112def9b211234bed663889486e55b95a5 → fd34a9565beb04c9cdab5ab8c97d64db3892c474 

Branch pushed to git repo; I updated commit sha1. New commits:
fd34a95  24420: move imports / name > names

comment:14 Changed 5 years ago by
Thanks. One last little thing: you don't use CompleteDiscreteValuationRings
.
comment:15 Changed 5 years ago by
Commit:  fd34a9565beb04c9cdab5ab8c97d64db3892c474 → bcf577dc4c384de604bb60577bdc08f53e7fba18 

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
bcf577d  24420: move imports / name > names

comment:18 Changed 5 years ago by
Branch:  u/vdelecroix/24420 → bcf577dc4c384de604bb60577bdc08f53e7fba18 

Resolution:  → fixed 
Status:  positive_review → closed 
New commits:
24420: clean laurent series