Updates:
Labels: -NeedsReview NeedsBetterPatch
Comment #20 on issue 1766 by Vinzent.Steinberg: expand(power_base=True) is
too aggressive
http://code.google.com/p/sympy/issues/detail?id=1766
Wow, you accumulated a great amount of changes in your 1766 branch! I
ignored the 2
'0000' commits. In general I'm +1 for the things implemented.
However I get several failures, see attachment. It's mainly about sets
which are
tried to be subscripted, due to the change to iterables.py.
Here some comments about the commits (assuming the tests would pass):
1785 looks fine.
1789: Why did you leave all the commented code? Just remove it using
another commit
to your branch, so we can get it back using git if we ever want. There is
also a very
long line, please move the long list elsewhere (outside the function?) and
break it.
For performance, a set could be used instead, but I'm not sure whether it
makes a
difference. Why is the a print statement in the test file? Looks fine
otherwise.
1778:I think we don't need decimal here and we could use mpmath, but that's
not
really important. And I'm not sure whether sympify() should use
nsimplify(). But it
looks fine.
For the hint manager: I think the foo(anything=True) should set anything
else to
False(overwrite the defaults). foo(something=False) should only change this
part,
although this is less likely to be needed. Same for foo(something=False,
anotherthing=True). But I think the first thing is much more intuitive.
I'm also not sure where to place it, but basic.py (manages calling etc.)
seems more
fitting to me than iterables.py. But it also fits to the utilities
directory.
1766: Somewhat large, could be splitted. For example the changes to
facts.py,
extract_additively, separate, trigsimp, separatevars, powsimp and
test_simplify.
Given the changes were not necessary to make tests pass. EXP should be a
dummy symbol.
Should we unify E**x and exp(x)?
The doctests for separate() could be preserved. There are some minor
formatting
issues like missing spaces around '=' in variable assignments (not kwargs
of course).
The new functionality looks great!
If commit 1772 fixes something, it lacks a test.
1794 looks fine.
1795 too, except the missing spaces around '==' and '+'. And there are
lines much too
long.
Attachments:
1766failures.txt 43.1 KB
--
You received this message because you are listed in the owner
or CC fields of this issue, or because you starred this issue.
You may adjust your issue notification preferences at:
http://code.google.com/hosting/settings
--
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.