Are there any other tests that fail if you make the change? I would look into the git history to see what changes added those tests if they made any justification for it. It seems to me that evaluate=False shouldn't be removing identities, but maybe there was a reason for it.
Aaron Meurer On Wed, Aug 23, 2017 at 3:58 AM, <[email protected]> wrote: > Hello everyone, > > It is my first post and I hope it is the right place to send my question. > First I would like to thank all the developers who created this great > library. We use it in my company but we encountered a very strange > behavior. I wanted to do a PR to correct it but the tests show that this > may be voluntary behavior. > > The problem : even with "evaluate=False" argument, Mul and Add remove > identity elements from its arguments. We can see this behavior in tests : > > https://github.com/sympy/sympy/blob/master/sympy/core/ > tests/test_arit.py#L1366 > > def test_suppressed_evaluation(): > a = Add(0, 3, 2, evaluate=False) > b = Mul(1, 3, 2, evaluate=False) > c = Pow(3, 2, evaluate=False) > assert a != 6 > assert a.func is Add > assert a.args == (3, 2) > assert b != 6 > assert b.func is Mul > assert b.args == (3, 2) > assert c != 9 > assert c.func is Pow > assert c.args == (3, 2) > > Removing arguments is already a kind of evaluation. For me, we should have > : > > def test_suppressed_evaluation(): > a = Add(0, 3, 2, evaluate=False) > b = Mul(1, 3, 2, evaluate=False) > # ... > assert a.args == (0, 3, 2) > # ... > assert b.args == (1, 3, 2) > # ... > > > and do not remove any arguments. > > I can do a PR to change this behavior (and update tests), It seems to me > that this can be corrected by moving the lines that remove identities > elements right after the "evaluate" processing : move the line > https://github.com/sympy/sympy/blob/master/sympy/core/operations.py#L31 > after the if on line 33. > > I prefer to ask the question here before because the presence of the tests > can make believe that this behavior is voluntary. > > Christophe Gabard > > -- > You received this message because you are subscribed to the Google Groups > "sympy" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to [email protected]. > To post to this group, send email to [email protected]. > Visit this group at https://groups.google.com/group/sympy. > To view this discussion on the web visit https://groups.google.com/d/ > msgid/sympy/2b59c125-f424-459b-b5a1-f220e5ef7db2%40googlegroups.com > <https://groups.google.com/d/msgid/sympy/2b59c125-f424-459b-b5a1-f220e5ef7db2%40googlegroups.com?utm_medium=email&utm_source=footer> > . > For more options, visit https://groups.google.com/d/optout. > -- You received this message because you are subscribed to the Google Groups "sympy" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected]. Visit this group at https://groups.google.com/group/sympy. To view this discussion on the web visit https://groups.google.com/d/msgid/sympy/CAKgW%3D6Ksa_bw%3DwTMOR79jLK6d%2Bphw4irAQ%3DUx8B944n4SqvADA%40mail.gmail.com. For more options, visit https://groups.google.com/d/optout.
