I have made a WIP PR for this change here : https://github.com/sympy/sympy/pull/13188
Hope all will be ok. On Wednesday, August 23, 2017 at 5:55:42 PM UTC+2, Aaron Meurer wrote: > > Regarding kernS, see the docstring of sympify(). It is related to > preventing automatic simplification due to distribution of constants. I > don't think it's supposed to prevent evaluation of constants, so your > commit looks like a regression in it. > > It may be helpful here to open a work in progress pull request from your > change, to make it easier to comment on it. > > Aaron Meurer > > On Wed, Aug 23, 2017 at 9:47 AM, <[email protected] <javascript:>> wrote: > >> I made a commit and add some comments about the changes : >> https://github.com/cgabard/sympy/commit/985e8e8251303aeed2763a2956a95f7d0f8f5ac2 >> >> There are several changes in addition to those already mentioned : >> >> - Some tests in sympy/core/tests/test_evaluate.py rely on >> `evaluate=False` needs to be updated, >> - `flatten` in sympy/core/mul.py and `fraction` in >> sympy/simplify/radsimp.py use `evaluate=False` and rely on the previous >> behavior. >> - *test about kernS* : I am not sure what the purpose of this function >> is, and I do not know what the actual behavior is. >> >> All tests are green on my computer with these modifications. >> >> >> On Wednesday, August 23, 2017 at 11:05:01 AM UTC+2, [email protected] >> wrote: >>> >>> 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]> 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] <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/dd559b97-3d48-4c6f-ab7a-491f4ac82fdb%40googlegroups.com >> >> <https://groups.google.com/d/msgid/sympy/dd559b97-3d48-4c6f-ab7a-491f4ac82fdb%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/9fbb3426-4742-4be3-ac3f-1205e6b66dc2%40googlegroups.com. For more options, visit https://groups.google.com/d/optout.
