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].
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.
For more options, visit https://groups.google.com/d/optout.

Reply via email to