Comment #14 on issue 1923 by nicolas.pourcelot: count_ops doesn't return a
count (by default)
http://code.google.com/p/sympy/issues/detail?id=1923
Sorry, I forgot to do "git fetch".
Ok, I can see your last version now.
Here's my first comments:
1. Some minor comment first:
In sympy/core/function.py
+ if type(expr) in (set, list, tuple):
+ if visual:
+ return C.Add(*[count_ops(a, visual) for a in expr])
+ ops = 0
+ for a in expr:
+ ops += count_ops(a, visual)
+ return ops
I would simply write something like this:
if type(expr) in (set, list, tuple):
return __builtin__.sum(count_ops(a, visual) for a in expr)
and add
import __builtin__
at the begining of sympy/core/function.py.
(By the way, I noticed that generator expressions are rarely used in sympy
code ; is this only for historical reasons ?)
2. There's an error with dict code:
>>> count_ops({x:y},visual=True)
UnboundLocalError: local variable 'k' referenced before assignment
Once again, I would write something like this:
if type(expr) is dict:
return __builtins__.sum(count_ops(a, visual=True) for a in
expr.iteritems()) # 'a' is a tuple
3.
+ # get the total op count
+ seq = [count_ops(a, visual=False) for a in expr.args]
+ seq.append(len(expr.args))
+ if expr.is_Add or expr.is_Mul or expr.is_Pow: # 1 + x: 2 args, 1 op
+ seq.append(-1)
+ ops = sum(seq)
In fact, I can't see the point of using a temporary list instead of adding
successive values:
ops = __builtin__.sum(count_ops(a, visual=False) for a in expr.args)
ops += len(expr.args))
if expr.is_Add or expr.is_Mul or expr.is_Pow: # 1 + x: 2 args, 1 op
ops -= 1
4. Just a design detail, but I liked .count_ops() being previously
implemented a a method ; imo it made it more easy and natural to customize
for special classes.
Maybe count_ops() function could be simply something like this:
def count_ops(expr, visual=False):
if type(expr) in (set, list, tuple):
return __builtin__.sum(count_ops(a, visual) for a in expr)
elif type(expr) is dict:
return __builtins__.sum(count_ops(a, visual=True) for a in
expr.iteritems())
elif not isinstance(expr, Expr):
if visual:
return S.Zero
return 0
return expr.count_ops()
and the remaining code could be put inside an Expr.count_ops() method ?
--
You received this message because you are subscribed to the Google Groups
"sympy-issues" group.
To post to this group, send email to sympy-iss...@googlegroups.com.
To unsubscribe from this group, send email to
sympy-issues+unsubscr...@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/sympy-issues?hl=en.
