Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add documentation to LaurentSeries point to accessors #39366

Draft
wants to merge 7 commits into
base: develop
Choose a base branch
from

Conversation

user202729
Copy link
Contributor

@user202729 user202729 commented Jan 22, 2025

I feel it's originally too undiscoverable. Since f and n are explicitly mentioned in the documentation I think it's a good idea to mention how to access it programmatically.

📝 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.

⌛ Dependencies

Copy link

github-actions bot commented Jan 28, 2025

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

@kwankyu
Copy link
Collaborator

kwankyu commented Feb 4, 2025

I don't know. The original docstring looks simple and to the point.

How about adding a short introduction to the head of the doc

https://doc-pr-39366--sagemath.netlify.app/html/en/reference/power_series/sage/rings/laurent_series_ring_element

where you can explain about f and n?

@user202729 user202729 marked this pull request as draft February 4, 2025 04:40
@kwankyu
Copy link
Collaborator

kwankyu commented Feb 6, 2025

Thanks.

I noticed that, in the doc, f is doubly used to represent a Laurent series and also to represent the power series part of its internal representation. To solve this, perhaps we may use a different name for the power series, like $f = t^n g$ where $g$ is a power series in $t$ of valuation zero.

Since your added explanation is fundamental, I suggest putting it to the very beginning of the doc, before the first example. You may also add an example to support it.

If you think the more work is out of the scope of your PR, just let me know. This is still an improvement. I may work upon it later.

@user202729
Copy link
Contributor Author

user202729 commented Feb 10, 2025

I think it's not that fundamental, but more like if someone need to implement some function that manipulates Laurent series and need to deconstruct it, it is the most efficient to use the accessors instead of, say, .list() or something.

Thus the original explanation being only in the "IMPLEMENTATION:" section. Of course if the implementation decides to be changed for some reason, .valuation() and .valuation_zero_part() would still stay there, but potentially not as efficient as a single method access.

But maybe like this? It's also possible to rename the valuation_zero_part to u to be consistent with the internal method name __u.

diff --git a/src/sage/rings/laurent_series_ring_element.pyx b/src/sage/rings/laurent_series_ring_element.pyx
index 758b7012240..211dab70de8 100644
--- a/src/sage/rings/laurent_series_ring_element.pyx
+++ b/src/sage/rings/laurent_series_ring_element.pyx
@@ -1,6 +1,16 @@
 r"""
 Laurent Series
 
+Laurent series in Sage are represented internally
+as a power of the variable times the unit part (which need not be a
+unit - it's a polynomial with nonzero constant term). The zero
+Laurent series has unit part 0.
+
+For a Laurent series `f` internally represented as `f = t^n \cdot g` where
+`t` is the variable and `g` has nonzero constant term, `g` can be accessed
+through :meth:`valuation_zero_part` and `n` can be accessed through
+:meth:`valuation`.
+
 EXAMPLES::
 
     sage: R.<t> = LaurentSeriesRing(GF(7), 't'); R
@@ -35,15 +45,6 @@ Saving and loading.
     sage: loads(K.dumps()) == K                                                         # needs sage.rings.real_mpfr
     True
 
-IMPLEMENTATION: Laurent series in Sage are represented internally
-as a power of the variable times the unit part (which need not be a
-unit - it's a polynomial with nonzero constant term). The zero
-Laurent series has unit part 0.
-
-For a Laurent series internally represented as `t^n \cdot f` where
-`t` is the variable, `f` can be accessed through :meth:`valuation_zero_part`
-and `n` can be accessed through :meth:`valuation`.
-
 AUTHORS:
 
 - William Stein: original version
@@ -93,8 +94,8 @@ cdef class LaurentSeries(AlgebraElement):
     r"""
     A Laurent Series.
 
-    We consider a Laurent series of the form `t^n \cdot f` where `f` is a
-    power series.
+    We consider a Laurent series of the form `f = t^n \cdot g` where `g` is a
+    power series with nonzero constant term.
 
     INPUT:
 

@kwankyu
Copy link
Collaborator

kwankyu commented Feb 10, 2025

Just one paragraph (without details uninteresting to users):

Laurent series in Sage are represented internally as a power of the variable
times the power series part. If a Laurent series `f` is represented as 
`f = t^n \cdot g` where `t` is the variable and `g` has nonzero constant term, 
`g` can be accessed through :meth:`valuation_zero_part` and `n` can be accessed 
through :meth:`valuation`.

?

or with u instead of g.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants