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]> wrote: > I made a commit and add some comments about the changes : > https://github.com/cgabard/sympy/commit/985e8e8251303aeed2763a2956a95f > 7d0f8f5ac2 > > 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/ms >>>> gid/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/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/CAKgW%3D6KhgwXgQ1wRwQoGVCpJ9FDpbLVi1QJBUwTqCJnTPASL5Q%40mail.gmail.com. For more options, visit https://groups.google.com/d/optout.
