Completion of polynomial rings and their fraction fields #40635

xcaruso · 2025-08-19T16:36:44Z

We implement the completion of polynomial rings and their fraction fields.
This results in rings of power/Laurent series.

📝 Checklist

The title is concise and informative.
The description explains in detail what this PR is about.
I have linked a relevant issue or discussion.
I have created tests covering the changes.
I have updated the documentation and checked the documentation preview.

github-actions · 2025-08-19T23:25:47Z

Documentation preview for this PR (built with commit 973dffa; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

xcaruso · 2025-08-20T09:51:18Z

@fchapoton @mantepse @tscrim
I will be happy to have your opinion on this PR, in particular concerning the three following points:

Is it reasonable to set a coercion map as I do? Could it break easily the coercion system?
I changed the behavior of PowerSeriesRing and LaurentSeriesRing to return lazy series when called with default_prec=Infinity. How does it sound? And did I implement this correctly?
I changed the behavior of _first_ngens to include generators of the base when the required number of generators exceeds that of the ring.

Of course, any other comment is welcome!

src/sage/rings/fraction_field.py

user202729 · 2025-08-20T11:03:41Z

src/sage/rings/polynomial/morphism.py

+from sage.rings.laurent_series_ring import LaurentSeriesRing
+
+
+class MorphismToCompletion(RingHomomorphism):


feels oddly specific? Can sage/rings/polynomial/polynomial_ring_homomorphism.pyx be reused?

You're probably right. I will try to refactor things differently.

src/sage/rings/polynomial/polynomial_ring.py

user202729 · 2025-08-20T11:07:26Z

src/sage/rings/power_series_ring.py

@@ -392,7 +392,7 @@ def PowerSeriesRing(base_ring, name=None, arg2=None, names=None,

    # the following is the original, univariate-only code

-    if isinstance(name, (int, integer.Integer)):
+    if isinstance(name, (int, integer.Integer)) or name is infinity:


I guess the comment above is not quite true? Also while there's the opportunity consider seeing if it's possible to avoid code copy paste.

Sorry, I don't really understand your point. What changes do you propose?

user202729 · 2025-08-20T11:09:54Z

src/sage/structure/category_object.pyx

@@ -370,13 +370,37 @@ cdef class CategoryObject(SageObject):
            sage: B.<z> = EquationOrder(x^2 + 3)                                        # needs sage.rings.number_field
            sage: z.minpoly()                                                           # needs sage.rings.number_field
            x^2 + 3
+
+        If the ring has less than `n` generators, the generators
+        of the base rings are happened recursively::


you mean "are appended".

Wait, why are we doing this?

Otherwise

sage: A.<x> = QQ[] sage: B.<u,a> = A.completion(x^2 + x + 1)

does not work because B._first_ngens(2) just returns u.

explain it in the documentation instead of here, I guess. But are you sure you don't want to just redefine _defining_names of the responsible class to return two objects instead?

Also, what are u and a here? If u = B(x^2+x+1) and a = B(x) (or vice versa) then… hm…

one can argue that B(x) alone generates the whole of B, so we could left that one alone and make an additional method that returns just the other item and assign that one separately.

explain it in the documentation instead of here, I guess. But are you sure you don't want to just redefine _defining_names of the responsible class to return two objects instead?

Well, the responsible class is PowerSeriesRing which is widely used, I'm not sure it's a good idea to change the behavior of _defining_names. (I agree that _first_ngens is also widely used, but it looks safer to me because we give the length of the list as an argument.)

Also, what are u and a here? If u = B(x^2+x+1) and a = B(x) (or vice versa) then… hm…

The completion is a ring of power series over a certain extension of the base ring; in my example, this extension is the number field $k = \mathbb Q[a]$ where $a$ is a root of the polynomial $x^2 + x + 1$. Let me underline that $a$ is an element of $B$ (of course), but it is not in $A$. It is what we call in general the Teichmüller representative of $x$.

On the other hand, $u$ is the variable of the power series ring; it is usually called the uniformizer; for polynomial rings, it can chosen to be simply $x - a$ and this is what I am doing.

So,

$A = ℚ[x]$

natural quotient map $A → k$ where $k = ℚ[x]/(x^2+x+1)$ sending $x$ to $a$ (here we introduce $a$)

$B = k〚u〛$ (here we introduce $u$)

natural injection $ℚ[x]/(x^2+x+1) → B$

composing the two sends $A[x]$ to $B$, the map has kernel $(x^2+x+1)$

doesn't sound right to me. I'd guess $A(x^2+x+1)$ to be $u$, not zero.

I'd say making a coercion is fine, but shouldn't this already exist as the composition of the two maps above? (does Sage figure things out by itself?)

Anyway typing an additional line $a = B(x)$ (sending $x$ through the composition of the two maps above) afterwards is easy enough, and if you put two generators in B.<u,a> it isn't entirely clear which is which, so I don't see this (potentially breaking) change too useful.

So,

$A = ℚ[x]$

natural quotient map $A → k$ where $k = ℚ[x]/(x^2+x+1)$ sending $x$ to $a$ (here we introduce $a$)

$B = k[[u]]$ (here we introduce $u$)

natural injection $ℚ[x]/(x^2+x+1) → B$

composing the two sends $A[x]$ to $B$, the map has kernel $(x^2+x+1)$

doesn't sound right to me. I'd guess $A(x^2 + x + 1)$ to be $u$, not zero.

The inclusion $A[x] \to B$ is not the composition of the two maps you described; it is the map that takes $x$ to $u+a$, i.e. $u = x - a$.

It's actually another option to define a map that takes $x^2 + x + 1$ to $u$ (in other words $x^2 + x + 1$ is also a uniformizer) but it's more complicated to implement and more costly.
Because we need to solve the equation $x^2 + x + 1 = u$ in $B$, in order to find the image of $x$.

user202729 · 2025-08-20T12:23:10Z

src/sage/rings/polynomial/polynomial_ring.py

+        INPUT:
+
+        - ``p`` (default: ``None``) -- an irreduclible polynomial or
+          ``Infiniity``; if ``None``, the generator of this polynomial


typo, too many i in Infiniity

xcaruso · 2025-08-20T14:38:56Z

The more I think about it, the more I am convinced that completion deserves their own class. Indeed, we would like some methods available, e.g. polynomial_ring* (to recover the underlying polynomial ring) or place(to recover the ideal at which we completed). Also the methodconstruction` does not reflect the construction.

The reason why I rejected this option at first is that it somehow breaks the unicity assumption for parents, in the sense that I will need to include A[x] and P in the key. In particular, we will have the following:

sage: A.<t> = QQ[]
sage: A1.<u> = A.completion(t - 1)
sage: A2.<u> = A.completion(t - 2)
sage: A1 is A2
False

although A1 and A2 are both power series ring in u over $\mathbb Q$.
Nonetheless, I underline that this does not work currently because it overloads the coercion graph.

So I finally think that having two different copies of PowerSeriesRing(QQ, name='u') is a lesser problem.
Moreover, this also solves the problem with _first_ngens because, here, I can simply redefine _defining_names.

What do you think about this?

user202729 · 2025-08-20T15:13:46Z

okay, makes sense. In that case, I suggest design the API as follows.

internally elements of B are represented by elements of C = PowerSeriesRing(A.quotient(f), 'u').
define a coercion morphism A → B sending x to u+a, where a = A.quotient(f)(x).
then B(f) is an uniformizer of B, although it likely would have infinitely many terms internally.
for c in C, write B._from_internal_repr(x) to be the element of B representing the above.

the above are what you did. The below aren't.

We must not expose the internal implementation detail above to the user.
For all they know, we might have sent $x$ to $-u+a$, and they wouldn't have noticed.
To do so, implement the following behavior:

Indexing
let $S ⊆ A$ be the subset of elements that is the image of the operation a ↦ a % f,
there's a natural set bijection (but not ring bijection!) between $S$ and A.quotient(f).
each element $b ∈ B$ can be represented uniquely as $b = c_0 + c_1 B(f) + c_2 B(f)^2 + ⋯$
then for all integer $i$, b[i] = c_i.

the % operation actually has a nice math description, namely the image are the polynomials
of degree less than $f$. For multivariate polynomial ring the choice is somewhat arbitrary
(based on Gröbner basis).

On the other hand, this indexing operation cannot be particularly efficiently implemented. We may want to implement .list() in almost-linear time, then tell the user to use that (to avoid quadratic behavior when indexing multiple elements), or for even more performance-critical code, they may access the internal detail in one way or the other.
String representation
Printing out $b$ must print it as c_0 + c_1 * f + c_2 * f^2 + O(f^n), similar to p-adic.
More coercion
there is a coercion A → B as above. There must not be a coercion B → C or vice versa.
Generator
B has a single generator which is the image of x over the coercion map.

Anyway, the above would require making separate parent and element classes.

xcaruso · 2025-08-20T15:27:34Z

I am not really in favor to choose $f$ as a uniformizer (i.e., as the variable of the power series ring) because, as I said, doing this transformation requires to solve the equation $f(x) = u$ in $B$ (which is certainly possible by Hensel's lemma but affects the complexity and the ease of implementation).
What's actually the problem with choosing another uniformizer?

I'm not also sure that x is the best choice for a generator. What is your argument for this?
In any case, we will need letters for the uniformizer, and the (Teichmüller representative of the) generator of the residue field, won't we?

xcaruso · 2025-08-20T15:28:11Z

I invite @roed314 to the discussion; he might have good suggestions.

user202729 · 2025-08-20T16:13:33Z

I think there was a typo somewhere.

Let $𝔭$ be the maximal ideal, then any element of $𝔭 ∖ 𝔭^2$ is an uniformizer.

You may send x to B._from_internal_repr(u+a). Then B(f) = B._from_internal_repr(f(u+a)) is also an uniformizer, apart from u.

(B(f) would be some polynomial in B._from_internal_repr(u) with zero constant term and finitely many nonzero coefficients then.)

we will need letters for the uniformizer

I guess you can give the user

.canonical_uniformizer() = B(f), string representation e.g. 1 * (x^2+x+1), somewhat complicated internal representation.
.efficient_uniformizer() = B._from_internal_repr(u), string representation depends on implementation detail.

No solving system of equation needed either way.

[edit: okay I guess Witt vectors display format may be more convenient in some settings, but mimicking the current behavior of Zp is more elementary, and you don't have to choose a generator name.]

the (Teichmüller representative of the) generator of the residue field

The element B(x) (after mapping to the residue field) is a generator of the residue field. This will just have string representation x.

[sorry, mistake. I guess you can provide a method for this, such thing would have representation like x + something * (x^2+x+1)^2 + ⋯, I guess. Internally it would be efficiently represented as B._from_internal_repr(a).]

I'm not also sure that x is the best choice for a generator. What is your argument for this?

Same thing happens in quotient of polynomial ring, e.g. in A.<x> = QQ[]; S.<xbar> = A.quotient(f), then xbar = S(x).

If the user wants some letter, I guess one can do something similar to print_mode in Zp constructor. Note that by default Zp elements directly use the prime p instead of any new character as well.

xcaruso · 2025-08-20T19:20:43Z

Let me summarize because I'm getting lost.

Let $K$ be a field and $A = K[x]$. Let also $\mathfrak p$ be an irreducible polynomial in $A$.
By definition, the completion of $A$ at $\mathfrak p$ is
$A_{\mathfrak p} := \varprojlim_n A/\mathfrak p^n$.
This gives a first way to handle $A_{\mathfrak p}$ and represent elements in there: an element is just a compatible sequence of elements of $A/\mathfrak p^n$ and we can work with them by just retaining the $n$-th element of the sequence (we then work at precision $\mathfrak p^n$).
It's basically what is done for the $p$-adics.

However, it turns out that $A_{\mathfrak p}$ is also isomorphic to a ring of power series with coefficients in the residue field $L := A/\mathfrak p$ (which is, of course, not the case for the $p$-adics). There exist actually many such isomorphisms: basically each choice of a uniformizer of $A_{\mathfrak p}$, i.e. an element of $\mathfrak p A_{\mathfrak p} \backslash \mathfrak p^2 A_{\mathfrak p}$ provides such an isomorphism (actually even several of them).

In what follows, I consider two of them. Denoting by $a$ the image of $x$ in $L$, here are they:
$\alpha : A_{\mathfrak p} \to L[[u]]$, $x \mapsto u + a$,
$\beta : A_{\mathfrak p} \to L[[v]]$, $\mathfrak p \mapsto v$.
It turns out that $\alpha$ is easy to compute since we know the image of $x$. On the other hand, computing $\beta$ requires to solve the equation $\mathfrak p(x) = v$ in $L[[v]]$ and, similarly, computing $\alpha^{-1}$ and $\beta^{-1}$ also requires to solve equations in order to compute the image of $a$ in $A_{\mathfrak p}$.

My proposal is that A.completion(p) just returns $L[[u]]$, so that we just need to compute $\alpha$ which defines the canonical inclusion $A \to L[[u]]$.

I'm not sure what's your proposal. My guess is that you would like to carry computations in $L[[u]]$ and print elements as in $L[[v]]$ (or maybe $A_{\mathfrak p}$, I'm not sure). Is it correct?
In any case, I think you need to compute $\alpha^{-1}$ (and possibly also $\beta$).

Of course, another different option would be to just implement operations in $A_{\mathfrak p}$ but I prefer not doing it because it requires a full new implementation and it will not be possible to take advantage of what is already implemented for power series.

xcaruso · 2025-08-20T19:28:08Z

Oh, I just noticed that computing $\alpha \circ \beta^{-1}$ is also easy because it acts trivially on $L$ and maps $v$ to $\mathfrak p(u+a)$. (However, going in the other direction looks more difficult.) Is it your point?

Let me also mention that in the $p$-adic case, although computing the completion of number fields is currently not implemented, it would also probably require at some point to provide names for the "unramified variable" and the "uniformizer" since general $p$-adic fields need them.

roed314 · 2025-08-20T19:35:27Z

I will try to take a look at the code and discussion this evening.

tscrim · 2025-08-21T01:22:23Z

Quick comments:

I think implementing the completion as a class is good. However, I don't think they need whole new class, but instead could be used on top of the existing (Lazy)PowerSeriesRing by an additional argument of the defining maximal ideal. I think this should be less work to implement.
Subsequently, having non-equal parents for different (i.e., nonequal) ideals is good. Operations between them don't (immediately) make sense. It also gives you a lot of control over the print representation.
I don't think _first_ngens should start pulling things from the base ring. It should just return all the generators of the ring at hand. I feel like otherwise it is breaking a (silent) promise.

xcaruso · 2025-08-26T13:22:03Z

I just pushed a rough implementation (without doctest so far) using RingExtension.
I think I am rather happy with the result but I'd like to have your feedback.

Here is a short demo:

sage: A.<x> = GF(7)[]
sage: Ap = A.completion(x^2 + 2*x + 2, prec=5)
sage: y = Ap(x)
sage: y
x
sage: y^20
5 + 4*x*(x^2 + 2*x + 2) + (5*x + 4)*(x^2 + 2*x + 2)^2 + (5*x + 5)*(x^2 + 2*x + 2)^3 + (6*x + 5)*(x^2 + 2*x + 2)^4 + ...
sage: y.inverse_of_unit()
(3*x + 6) + (5*x + 3)*(x^2 + 2*x + 2) + (6*x + 5)*(x^2 + 2*x + 2)^2 + (3*x + 6)*(x^2 + 2*x + 2)^3 + (5*x + 3)*(x^2 + 2*x + 2)^4 + O((x^2 + 2*x + 2)^5)
sage: a = y.teichmuller()
sage: a
x + (4*x + 4)*(x^2 + 2*x + 2) + (3*x + 3)*(x^2 + 2*x + 2)^2 + (6*x + 6)*(x^2 + 2*x + 2)^3 + ...
sage: a^2 + 2*a + 2
0

We can also recover the underlying power series ring as follows:

sage: Ap.power_series(names=('u','a'))
Power Series Ring in u over Finite Field in a of size 7^2

However, the construction S.<u,a> = Ap.power_series() does not work because _first_ngens only returns one generator. I continue to think to it would be nice to modify this.

xcaruso · 2025-08-27T08:44:17Z

Finally, I used the method quotient instead of extension; the advantage is that it does not require a variable name since it uses xbar as a default.
Hence now the following works:

sage: S.<u> = Ap.power_series()
sage: S
Power Series Ring in u over Univariate Quotient Polynomial Ring in xbar over Finite Field of size 7 with modulus x^2 + 2*x + 2

user202729 · 2025-08-27T10:00:27Z

Looks nice, though I haven't looked too carefully.

I wonder what's a good way out of this. Maybe refactor the code such that

A.<b, c> = f()

desugar to

A, (b, c) = objgens(f(names=('b', 'c')));

then f can return a special object with .objgens()[0] does not return self to "workaround" the issue.

Caveat being currently .<> sugar allows specifying less generator than .gens(), but it's not clear why should anyone want to use this. (e.g. R.<x> = QQ['x', 'y'])

Maybe

A, (b, c) = _objgens_allow_less_generators(f(names=('b', 'c')), 2);

…etion

xcaruso · 2025-08-28T17:53:40Z

A question: each time, I change one line in a pyx file, sage -b/make build recompiles all the Sage library; do you know why and what should I do to prevent this?

fchapoton · 2025-08-28T18:01:48Z

Yes, this is terrible. This happens for everybody. This is supposed to be cured by the switch to meson in #39030

user202729 · 2025-08-29T00:17:21Z

you can already use meson using conda/mamba (I recommend mamba as it's faster to install). See https://doc.sagemath.org/html/en/installation//meson.html

Anyway, I think make build also have incremental build, which is why you (at least, I did) see the incremental compilation time to be 2 minutes instead of 5 hours. But for meson, it could be as little as 5 seconds.

fchapoton · 2025-08-30T07:38:44Z

Il faut pas de return dans une methode __init__

user202729 · 2025-08-30T08:36:02Z

src/sage/categories/pushout.py

@@ -2777,9 +2777,9 @@ def commutes(self, other):
        functors in opposite order works. It does::

            sage: P.<x> = ZZ[]
-            sage: C = P.completion(x).construction()[0]
+            sage: C = P.completion('x').construction()[0]


does P.completion(x) no longer work? It ought to work right?

P.completion(x) works but it now returns a CompletionPolynomialRing.
This doctest actually fails without the modification but IMO, it is the fault of the doctest that pretends that completion commutes with localization, which is not true.
In this precise example, for example, the fraction field of the completion of $\mathbb Z[x]$ is a strict subfield of $\mathbb Q((x))$ (= the completion of the fraction field of $\mathbb Z[x]$).

user202729 · 2025-08-30T08:36:11Z

src/sage/categories/pushout.py

@@ -2541,7 +2541,7 @@ def __init__(self, p, prec, extras=None):
            from sage.rings.infinity import Infinity
            if self.p == Infinity:
                if self.type not in self._real_types:
-                    raise ValueError("completion type must be one of %s" % (", ".join(self._real_types)))
+                    raise ValueError("completion type must be one of %s" % (", ".join(self._real_types[1:])))


When can the type be None?

When called with a polynomial ring and Infinity (which was not implemented beforehand).

user202729 · 2025-08-30T08:38:05Z

src/sage/rings/big_oh.py

+        if x.denominator().is_one():
+            x = x.numerator()
+        elif x.numerator().is_one():
+            x = x.denominator()


This feels fishy, what if the element is like (x^2+1)^(-2)?

In this case, the numerator is $1$, no?
The point is that, if we have at the same time a numerator and a denominator, then it's not a valid completion.

user202729 · 2025-08-30T08:39:00Z

src/sage/rings/big_oh.py

-            raise NotImplementedError("completion only currently defined "
-                                      "for the maximal ideal (x)")
+            p, n = x.perfect_power()
+            C = A.completion(p)


Add a test where p is not a perfect power of an irreducible and ensure that an error is raised.

Maybe also add a test where elements from two different parents are added, make sure they are not implicitly coercible.

Actually, for functoriality, it is nice to have completion at nonmaximal ideals.

user202729 · 2025-08-30T08:40:40Z

src/sage/rings/completion_polynomial_ring.py

+
+    def teichmuller_lift(self):
+        r"""
+        Return the Teichmüller representative of this element.


do you think it makes sense to let this method be called on arbitrary element of Ap e.g. Ap((x^2+x+1) + x).teichmuller_lift()?

I would say yes: any element has a Teichmüller lift.

user202729 · 2025-08-30T08:42:05Z

src/sage/rings/completion_polynomial_ring.py

+
+        If the parent of this element does not contain elements
+        of negative valuation, the expansion starts at `0`;
+        otherwise, it starts at the valuation of the elment.


Typo, "el[e]ment".

Also what does expansion() do for p-adic and Laurent series at the moment? This feels strange.

It is the same behavior for the $p$-adics (except that we have a flag start_val to change the behavior).
expansion is not implemented for power series ring.

user202729 · 2025-08-30T08:50:29Z

src/sage/rings/polynomial/polynomial_element.pyx

+        for e, m in self.degree().factor():
+            for _ in range(m):
+                try:
+                    f = f.nth_root(e)


it's not clear this algorithm is correct. For example if self = x^4, you would try to first self.nth_root(2), which for all you know might give -x^2, then you run nth_root(2) again, if the base field is ℚ then there wouldn't be a solution.

Yes, good point! Thanks!

user202729 · 2025-08-30T08:52:32Z

src/sage/rings/power_series_pari.pyx

@@ -434,7 +434,7 @@ cdef class PowerSeries_pari(PowerSeries):
            if a.valuation() <= 0:
                raise ValueError("can only substitute elements of positive valuation")
        elif isinstance(Q, PolynomialRing_generic):
-            Q = Q.completion(Q.gen())
+            Q = Q.completion(Q.variable_name())


I wonder if this can be seen as a breaking change (previously it returns a power series, now it returns a CompletionPolynomialRing). The alternative is to special case such as if p == self.gen():

No: Q.completion('x') (with a string) still returns a PowerSeriesRing; it's useful in particular for functoriality.

However, I would special case if the generator is the input.

user202729 · 2025-08-30T08:53:14Z

src/sage/rings/ring_extension.pyx

@@ -593,7 +594,7 @@ cdef class RingExtension_generic(Parent):
        # Some checks
        if (base not in CommutativeRings()
         or ring not in CommutativeRings()
-         or not defining_morphism.category_for().is_subcategory(CommutativeRings())):
+         or not defining_morphism.category_for().is_subcategory(Rings())):


Why are we doing this?

For some reason I was not able to understand, a certain coercion morphism (or maybe a composition of two of them) is in the category of Rings and not CommutativeRings.
Probably, this should be fixed but, in any case, since any ring morphism between commutative rings is certainly a morphism of commutative rings, I think it's very safe to do this change.

That definitely seems like a bug and deserves its own issue (which should also be referenced here I think).

tscrim · 2025-09-01T00:12:05Z

The point is that the natural map A → L [ [ u ] ] takes x to a + u and so it is not the composite of the two coercion maps A → L → L [ [ u ] ] which takes x to a . (Here L = A / p , see my previous messages.)

Okay, thank you for the summary. As a reminder $A = K[x]$. While the underlying model for this would be $L[[u]]$, the actual base ring (field) for the parent should be $K$ in this case as we are thinking of this as a $K$-algebra, right? So there would be no automatic coercion from $L$. If instead you wanted to implement it with the base ring being $L$, you can disable the automatic base ring coercion (or set it to a different map) by using _coerce_map_from_base_ring().

tscrim · 2025-09-01T00:46:27Z

src/sage/rings/ring_extension.pyx

@@ -593,7 +594,7 @@ cdef class RingExtension_generic(Parent):
        # Some checks
        if (base not in CommutativeRings()
         or ring not in CommutativeRings()
-         or not defining_morphism.category_for().is_subcategory(CommutativeRings())):
+         or not defining_morphism.category_for().is_subcategory(Rings())):


That definitely seems like a bug and deserves its own issue (which should also be referenced here I think).

tscrim · 2025-09-01T00:47:58Z

src/sage/rings/polynomial/polynomial_ring.py

+          with the construction mecanism
+
+        EXAMPLES::


Suggested change

with the construction mecanism

EXAMPLES::

with the construction mechanism

.. SEEALSO::

:mod:`sage.rings.completion_polynomial_ring`

EXAMPLES::

tscrim · 2025-09-01T00:50:10Z

src/sage/rings/laurent_series_ring.py

@@ -221,12 +221,14 @@ def __classcall__(cls, *args, **kwds):
            'q'
        """
        from .power_series_ring import PowerSeriesRing
-
+        if 'default_prec' in kwds and kwds['default_prec'] is infinity:


Suggested change

if 'default_prec' in kwds and kwds['default_prec'] is infinity:

if kwds.get('default_prec', None) is infinity:

Personal style choice here.

tscrim · 2025-09-01T00:50:49Z

src/sage/rings/fraction_field.py

+          previous variable names
+
+        EXAMPLES::


Suggested change

previous variable names

EXAMPLES::

previous variable names

.. SEEALSO::

:mod:`sage.rings.completion_polynomial_ring`

EXAMPLES::

tscrim · 2025-09-01T00:53:10Z

src/sage/rings/completion_polynomial_ring.py

+Completion of polynomial rings and their fraction fields
+"""


Suggested change

Completion of polynomial rings and their fraction fields

"""

Completion of polynomial rings and their fraction fields

AUTHORS:

- Xavier Caruso ([INSERT A DATE]): Initial implementation

"""

tscrim · 2025-09-01T00:59:41Z

src/sage/rings/completion_polynomial_ring.py

+            Completion of Univariate Polynomial Ring in x over Integer Ring at x
+        """
+        if isinstance(ring, PolynomialRing_generic):
+            if isinstance(p, str):


Suggested change

if isinstance(p, str):

if isinstance(p, str) or p == ring.gen():

As mentioned on another comment, I think it is good to special case this.

On the other hand, I think it is important to have something uniform (with the same parent, the same class) when the ideal varies.
It could be a mess if you do

p = <complicated calculation> Ap = A.completion(p)

and the result you get depends on the fact that the result of the complicated calculation is or is not the generator.
Another issue concerns inexact base rings when any approximation of a generator, in particular O(1)*x will be recognized a generator.

Then power series rings should be subclasses. The point is that there we can take advantage of the specific input, so we can and should optimize it (and have other things like natural coercions). Although as long as the API is the same, ducktyping will make it okay, but this would be a code smell to me. Also the output type depends on the precision, so there might be a bigger refactoring needed...

tscrim · 2025-09-01T01:00:46Z

src/sage/rings/completion_polynomial_ring.py

+        """
+        if isinstance(ring, PolynomialRing_generic):
+            if isinstance(p, str):
+                return PowerSeriesRing(ring.base_ring(),


I think you also need to handle the case of default_prec == oo here, right?

It's done in the function PowerSeriesRing.

tscrim · 2025-09-01T01:01:56Z

src/sage/rings/completion_polynomial_ring.py

+            A = ring
+        elif isinstance(ring, FractionField_1poly_field):
+            if isinstance(p, str):
+                return LaurentSeriesRing(ring.base_ring(),


Won't this be wrong if the base ring is a ring? For example, $\mathbb{Z}(x)$ (which is isomorphic to $\mathbb{Q}(x)$ I believe) would have $\mathbb{Z}((x))$, but that is strictly smaller than $\mathbb{Q}((x))$ (e.g., $\exp(x)$ is not in the former).

tscrim · 2025-09-01T01:05:22Z

src/sage/rings/completion_polynomial_ring.py

+    def laurent_series_ring(self, names=None, sparse=None):
+        r"""
+        Return a Laurent series ring, which is isomorphic to
+        the fraction field of this completion.


Again, I think we need to be a bit careful with the base ring.

tscrim · 2025-09-01T01:05:36Z

src/sage/rings/completion_polynomial_ring.py

+    def is_integral_domain(self):
+        return self._residue_ring.is_integral_domain()


Another doctest reminder.

completion of polynomial rings and their fraction fields

2368519

github-actions bot added the s: needs review label Aug 19, 2025

file morphism.py

573dae0

Xavier Caruso added 3 commits August 20, 2025 08:54

fix failing doctests

ce72980

fix doctest

d990b8f

avoid recursive loop in coercion

eff3e22

user202729 reviewed Aug 20, 2025

View reviewed changes

src/sage/rings/fraction_field.py Outdated Show resolved Hide resolved

user202729 reviewed Aug 20, 2025

View reviewed changes

src/sage/rings/polynomial/polynomial_ring.py Outdated Show resolved Hide resolved

user202729 reviewed Aug 20, 2025

View reviewed changes

Xavier Caruso added 2 commits August 20, 2025 13:49

address easy remarks by user202729

2f06fbc

typo

e6a8942

user202729 reviewed Aug 20, 2025

View reviewed changes

Xavier Caruso added 3 commits August 20, 2025 14:32

remove class MorphismToCompletion

5f008e6

typo

7468d4a

documentation of the function morphism_to_completion

779cb91

xcaruso marked this pull request as draft August 21, 2025 09:11

github-actions bot removed the s: needs review label Aug 21, 2025

implementation using RingExtension

6791145

Xavier Caruso added 2 commits August 26, 2025 15:22

fix default precision

dede4c3

use quotient instead of extension

e31740c

set up correctly the generator

d0bf69c

Xavier Caruso added 3 commits August 27, 2025 19:32

Merge branch 'develop' of https://github.com/sagemath/sage into compl…

1177266

…etion

completion of rational function field

b2ee24f

infinite place

4dea431

Xavier Caruso added 2 commits August 28, 2025 21:09

bigoh

0f8b78d

implement quasi-linear algorithm for expansion (and printing)

c84ffe6

Xavier Caruso added 2 commits August 29, 2025 19:33

doctests

ee59022

completion -> completion_polynomial_ring

d816dc5

oops

973dffa

user202729 reviewed Aug 30, 2025

View reviewed changes

tscrim requested changes Sep 1, 2025

View reviewed changes

		from sage.rings.laurent_series_ring import LaurentSeriesRing


		class MorphismToCompletion(RingHomomorphism):

	if 'default_prec' in kwds and kwds['default_prec'] is infinity:
	if kwds.get('default_prec', None) is infinity:

	if isinstance(p, str):
	if isinstance(p, str) or p == ring.gen():

		def is_integral_domain(self):
		return self._residue_ring.is_integral_domain()

Uh oh!

Completion of polynomial rings and their fraction fields #40635

Are you sure you want to change the base?

Completion of polynomial rings and their fraction fields #40635

Uh oh!

Conversation

xcaruso commented Aug 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📝 Checklist

Uh oh!

github-actions bot commented Aug 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xcaruso commented Aug 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xcaruso Aug 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xcaruso Aug 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xcaruso Aug 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xcaruso commented Aug 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

user202729 commented Aug 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xcaruso commented Aug 20, 2025

Uh oh!

xcaruso commented Aug 20, 2025

Uh oh!

user202729 commented Aug 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xcaruso commented Aug 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xcaruso commented Aug 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

roed314 commented Aug 20, 2025

Uh oh!

tscrim commented Aug 21, 2025

Uh oh!

xcaruso commented Aug 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xcaruso commented Aug 27, 2025

Uh oh!

user202729 commented Aug 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xcaruso commented Aug 28, 2025

xcaruso commented Aug 19, 2025 •

edited

Loading

github-actions bot commented Aug 19, 2025 •

edited

Loading

xcaruso commented Aug 20, 2025 •

edited

Loading

xcaruso Aug 20, 2025 •

edited

Loading

xcaruso Aug 20, 2025 •

edited

Loading

xcaruso Aug 20, 2025 •

edited

Loading

xcaruso commented Aug 20, 2025 •

edited

Loading

user202729 commented Aug 20, 2025 •

edited

Loading

user202729 commented Aug 20, 2025 •

edited

Loading

xcaruso commented Aug 20, 2025 •

edited

Loading

xcaruso commented Aug 20, 2025 •

edited

Loading

xcaruso commented Aug 26, 2025 •

edited

Loading

user202729 commented Aug 27, 2025 •

edited

Loading

user202729 commented Aug 29, 2025 •

edited

Loading

fchapoton commented Aug 30, 2025 •

edited

Loading

xcaruso Aug 30, 2025 •

edited

Loading

xcaruso Aug 30, 2025 •

edited

Loading