Opened 12 years ago
Closed 11 years ago
#10272 closed defect (fixed)
laurent series truncate behaviour different from power series truncate
Reported by: | Timo Kluck | Owned by: | Martin Albrecht |
---|---|---|---|
Priority: | minor | Milestone: | sage-5.1 |
Component: | commutative algebra | Keywords: | laurent series, truncate sd40.5 |
Cc: | Merged in: | sage-5.1.beta3 | |
Authors: | Timo Kluck | Reviewers: | Marshall Hampton, Jeroen Demeyer, Karl-Dieter Crisman |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
There is the following difference in behaviour between truncate() for power series and laurent series:
sage: P.<p> = PowerSeriesRing(QQ) sage: L.<l> = LaurentSeriesRing(QQ) sage: f = p^2 + p^3 + p^5 +p^6 + O(p^7) sage: g = l^2 + l^3 + l^5 +l^6 + O(l^7) sage: f.truncate(6) p^5 + p^3 + p^2 sage: g.truncate(6) l^2 + l^3 + l^5 + O(l^6) sage: f.truncate_powerseries(6) p^2 + p^3 + p^5 + O(p^6)
The problem is simply that g.truncate() calls truncate_powerseries() on its underlying power series.
The attached patch changes LaurentSeries?.truncate() such that is calls truncate() on the underlying powerseries. For the current behaviour, it also adds a method truncate_laurentseries() which calls truncate_powerseries() on the underlying powerseries.
In my opinion, the methods truncate_powerseries and truncate_laurentseries are superfluous since they are equivalent to add_bigoh().
Attachments (1)
Change History (19)
comment:1 Changed 12 years ago by
Description: | modified (diff) |
---|
comment:2 Changed 12 years ago by
comment:3 Changed 12 years ago by
Status: | new → needs_review |
---|
comment:5 Changed 12 years ago by
Status: | needs_review → positive_review |
---|
Looks good, positive review. Doesn't break any doctests in rings because Laurent series are so poorly tested! Only bad thing is that you should include the ticket number on the patch, i.e. this one should have been named something like trac_10272_laurent_series_truncate.patch. This helps the release manager and others in a variety of ways.
comment:6 Changed 12 years ago by
Reviewers: | → Marshall Hampton |
---|
comment:7 Changed 12 years ago by
Milestone: | → sage-4.6.2 |
---|
comment:8 Changed 12 years ago by
Status: | positive_review → needs_work |
---|
Patches should be created using hg export tip
, not hg diff
. I also remind you that a patch should have a sensible commit message (use hg qrefresh -e
for that) and that the ticket number should appear on the first line of the commit message.
comment:9 Changed 12 years ago by
Status: | needs_work → needs_review |
---|
I just added the patch in the right format, i.e. the output of hg_sage.export()
instead of a normal diff.
comment:10 Changed 12 years ago by
Status: | needs_review → needs_work |
---|---|
Work issues: | → docstring |
Please read http://sagemath.org/doc/developer/conventions.html#documentation-strings and fix the documentation of this new function accordingly. In particular, you should add at least one example.
comment:11 Changed 12 years ago by
Status: | needs_work → needs_review |
---|
Thanks, just added docstrings and examples.
comment:12 Changed 12 years ago by
Reviewers: | Marshall Hampton → Marshall Hampton, Jeroen Demeyer |
---|---|
Status: | needs_review → positive_review |
comment:13 Changed 12 years ago by
Status: | positive_review → needs_work |
---|---|
Work issues: | docstring → rebase |
This needs to be rebased:
applying /scratch/jdemeyer/merger/downloads/trac_10272_laurent_series_truncate.patch patching file sage/rings/laurent_series_ring_element.pyx Hunk #1 FAILED at 725 1 out of 1 hunks FAILED -- saving rejects to file sage/rings/laurent_series_ring_element.pyx.rej abort: patch failed to apply
Changed 11 years ago by
Attachment: | trac_10272_laurent_series_truncate.patch added |
---|
comment:14 Changed 11 years ago by
Status: | needs_work → needs_review |
---|
Sorry for not having done this earlier, I totally forgot. I just uploaded a new version that should apply cleanly against the current version. Please review.
comment:15 Changed 11 years ago by
Reviewers: | Marshall Hampton, Jeroen Demeyer → Marshall Hampton, Jeroen Demeyer, Karl-Dieter Crisman |
---|---|
Status: | needs_review → positive_review |
sage -t "devel/sage-main/sage/rings/laurent_series_ring_element.pyx" [2.1 s] ---------------------------------------------------------------------- All tests passed! Total time for all tests: 2.1 seconds
And it still applies.
comment:16 Changed 11 years ago by
Keywords: | sd40.5 added |
---|
comment:17 Changed 11 years ago by
Work issues: | rebase |
---|
comment:18 Changed 11 years ago by
Merged in: | → sage-5.1.beta3 |
---|---|
Resolution: | → fixed |
Status: | positive_review → closed |
Is this ready for review? If so, you should change its status to "needs review".