#9652: Unnecesary and buggy code in arith.py
------------------------+---------------------------------------------------
Reporter: mderickx | Owner: AlexGhitza
Type: defect | Status: needs_review
Priority: minor | Milestone:
Component: algebra | Keywords:
Author: | Upstream: N/A
Reviewer: | Merged:
Work_issues: |
------------------------+---------------------------------------------------
Comment(by mstreng):
Two questions:
* Is there any case in which any code after `return infinity` is executed
and succesful?
* Is there any case in which any code after `return m.valuation(...)` is
executed and useful?
Can't you remove a lot more while you are at it? The code seems to be
there to catch off weird unexpected cases that don't have a valuation
attribute, but
1. if the case is weird enough, then nothing guarantees that this
function terminates
2. if the case happens to have a negative valuation, then the function
returns 0 instead of a negative number
While you are at it, I found that the function doesn't handle the
valuation attribute of power series well (because it doesn't allow you to
specify the (unique) prime p).
{{{
x = QQ[['x']].gen()
valuation(x^2, x)
TypeError: valuation() takes no arguments (1 given)
valuation(x^2)
TypeError: valuation() takes exactly 2 arguments (1 given)
}}}
The first error is because `PowerSeries.valuation()` takes no arguments;
the second is because the global function takes exactly 2 arguments.
How about defining the global function simply as follows?
{{{
def valuation(m, *args1, **args2):
return m.valuation(*args1, **args2)
}}}
or
{{{
def valuation(m, *args1, **args2):
if m == 0:
return sage.rings.all.infinity
return m.valuation(*args1, **args2)
}}}
I haven't tried and/or ran any doctests with this, but anything that is
broken by this isn't good coding practice, as you said :) Plus this fixes
my example with `valuation(x^2)`.
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/9652#comment:2>
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.
