#9652: Unnecesary and buggy code in arith.py
------------------------+---------------------------------------------------
Reporter: mderickx | Owner: AlexGhitza
Type: defect | Status: new
Priority: minor | Milestone:
Component: algebra | Keywords:
Author: | Upstream: N/A
Reviewer: | Merged:
Work_issues: |
------------------------+---------------------------------------------------
This code was added in #1148. I really think that the lines removed in my
patch should be gone. The current (4.4.4) code is:
{{{
def valuation(m, p):
if hasattr(m, 'valuation'):
return m.valuation(p)
if is_FractionFieldElement(m):
return valuation(m.numerator()) - valuation(m.denominator())
if m == 0:
import sage.rings.all
return sage.rings.all.infinity
r = 0
power = p
while not (m % power): # m % power == 0
r += 1
power *= p
return r
}}}
Putting implementation specific to Fraction fields in a global function is
bad practice. And since fraction fields have an implementation of
valuation part of the above code will not be execucted. If it magicaly
get's excecuted it will return bad results since it doesn't take into
account the input variable "p" as it should.
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/9652>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/sage-trac?hl=en.
