Opened 8 years ago
Closed 7 years ago
#10272 closed defect (fixed)
laurent series truncate behaviour different from power series truncate
Reported by: | tkluck | Owned by: | malb |
---|---|---|---|
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 8 years ago by
- Description modified (diff)
comment:2 Changed 8 years ago by
comment:3 Changed 8 years ago by
- Status changed from new to needs_review
comment:4 Changed 8 years ago by
Thanks, I just did.
comment:5 Changed 8 years ago by
- Status changed from needs_review to 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 8 years ago by
- Reviewers set to Marshall Hampton
comment:7 Changed 8 years ago by
- Milestone set to sage-4.6.2
comment:8 Changed 8 years ago by
- Status changed from positive_review to 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 8 years ago by
- Status changed from needs_work to 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 8 years ago by
- Status changed from needs_review to needs_work
- Work issues set to 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 8 years ago by
- Status changed from needs_work to needs_review
Thanks, just added docstrings and examples.
comment:12 Changed 8 years ago by
- Reviewers changed from Marshall Hampton to Marshall Hampton, Jeroen Demeyer
- Status changed from needs_review to positive_review
comment:13 Changed 8 years ago by
- Status changed from positive_review to needs_work
- Work issues changed from docstring to 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 7 years ago by
comment:14 Changed 7 years ago by
- Status changed from needs_work to 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 7 years ago by
- Reviewers changed from Marshall Hampton, Jeroen Demeyer to Marshall Hampton, Jeroen Demeyer, Karl-Dieter Crisman
- Status changed from needs_review to 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 7 years ago by
- Keywords sd40.5 added
comment:17 Changed 7 years ago by
- Work issues rebase deleted
comment:18 Changed 7 years ago by
- Merged in set to sage-5.1.beta3
- Resolution set to fixed
- Status changed from positive_review to closed
Is this ready for review? If so, you should change its status to "needs review".