Opened 14 years ago
Closed 14 years ago
#1148 closed enhancement (fixed)
[with patch, with positive review] valuation doesn't work for rational numbers
Reported by: | dmharvey | Owned by: | dmharvey |
---|---|---|---|
Priority: | minor | Milestone: | sage-2.8.15 |
Component: | basic arithmetic | Keywords: | |
Cc: | Merged in: | ||
Authors: | Reviewers: | ||
Report Upstream: | Work issues: | ||
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
It would be nice if valuation(3/5, 5)
returned -1, but it does this:
sage: valuation(3/5, 5) --------------------------------------------------------------------------- <type 'exceptions.ZeroDivisionError'> Traceback (most recent call last) /Users/david/series/<ipython console> in <module>() /Users/david/sage-2.8.12/local/lib/python2.5/site-packages/sage/rings/arith.py in valuation(m, p) 425 r=0 426 power=p --> 427 while m%power==0: 428 r += 1 429 power *= p /Users/david/series/rational.pyx in sage.rings.rational.Rational.__mod__() /Users/david/series/integer.pyx in sage.rings.integer.Integer.inverse_mod() <type 'exceptions.ZeroDivisionError'>: Inverse does not exist.
Attachments (2)
Change History (7)
comment:1 Changed 14 years ago by
- Milestone set to sage-2.8.13
comment:2 Changed 14 years ago by
- Owner changed from somebody to dmharvey
- Status changed from new to assigned
Changed 14 years ago by
comment:3 Changed 14 years ago by
- Summary changed from valuation doesn't work for rational numbers to [with patch] valuation doesn't work for rational numbers
Changed 14 years ago by
comment:4 Changed 14 years ago by
- Summary changed from [with patch] valuation doesn't work for rational numbers to [with patch, with positive review] valuation doesn't work for rational numbers
I tried the combination of 1148.hg and 1148-more.patch patch against 2.8.14. The source code looks reasonable, and doctesting arith.py and rational.pyx (the two files touched by the change) both succeed. (I did not try testall.)
In other words, looks good to me.
comment:5 Changed 14 years ago by
- Resolution set to fixed
- Status changed from assigned to closed
Merged in 2.8.15.alpha1.
Note: See
TracTickets for help on using
tickets.
fixed it