Hum, tests are currently running on my computer and some to fail. Some only need updates, like sympy/core/tests/test_evaluate.py which explicitly refer to evaluate = False current behavior. Some other tests seem to fail without apparent reasons. I will investigate this before doing a PR.
Thank you for your reply, Christophe Gabard On Wednesday, August 23, 2017 at 10:48:47 AM UTC+2, Aaron Meurer wrote: > > 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] <javascript:>> 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] <javascript:>. >> To post to this group, send email to [email protected] <javascript:> >> . >> 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/2ce88410-b900-4dd3-aa73-37a6935b61cd%40googlegroups.com. For more options, visit https://groups.google.com/d/optout.
