Fix 14796

[DarylMcCullough]

Allow substitution of function symbols within an integral

References to other Issues or PRs

Fixes #14796

Brief description of what is fixed or changed

In the code for substitutions within an expression involving limits (such as an
integral), the code breaks if you try to substitute one function symbol for another.
The reason for this is found in the file sympy/concrete/expr_with_limits.py in
the implementation of the method _eval_subs(self, old, new) of the class ExprWithLimits. In
that method, the properties and methods .free_symbols, .args, .atoms are consulted.
However, those properties and methods do not return list (or iterables) in the case
where the old or new arguments are instances of FunctionClass.

A fix that works in the case that sparked the issue is to check if those properties
and methods return lists, and if not, proceed as if they returned empty lists. This
perhaps should be implemented in the FunctionClass.

Other comments

The subs function allows substitutions of subexpressions within an expression.
So for example:

from sympy import *
x, y = symbols("x y")
f, g = symbols("f g", cls=Function)
expr = f(x)
expr1 = expr.subs(x, y)

Afterwards, expr1 will be the expression f(y). You can even substitute function
symbols for other function symbols:

expr2 = expr.subs(f, cos)

Afterwards, expr2 will be the expression cos(x). However, if you try to do
the same substitution within an integral, Integral(f(x), (x,0,1)), (the integral
from x=0 to x=1 of f(x)) then instead of getting Integral(cos(x), (x,0,1), the
code raises an error. This is because in substituting cos for f, it checks
the free variables of f and the args of f and the atoms of cos, and those
methods and properties are not defined on FunctionClass objects.

[asmeurer]

I'm not sure if subs should be doing this. We already have replace that does this. What do others think?

[asmeurer]

Oh I guess expr.subs(f, g) already does work for normal expressions. This just adds it to Integral. Is f(x).subs(f, g) -> g(x) intentional behavior or just accidental?

[asmeurer]

Remove this comment. The first part will be in the git history, and also the AUTHORS file. The second part should be a comment here on the pull request.

Regarding the question, we apparently allow things like Integral(f(x), f(x)). Does this do the right thing for it?

[asmeurer]

Look at the test failure in Travis. You need to remove trailing whitespace.

Also, this needs to have a test added.

[Abdullahjavednesar]

A blank line required at the end.

[DarylMcCullough]

This is strange. I'm pretty sure that the build failing was not due to anything I did. When I did tests locally, using ./setup.py tests, everything checked out.

[asmeurer]

If the ability to subs functions really is intentional, I wonder if it should be done in subs itself, instead of requiring every _eval_subs method to support it.

[asmeurer]

So it looks like this does work because of Application._eval_subs, and looking through the history, it's been there for a while. So we should probably keep the functionality. But I do think it does need to be in subs, not _eval_subs. The _eval_subs methods should be able to assume that old and new are Basic subclasses. So my suggestion would be to

• Remove this change (but keep the tests)
• Remove Application._eval_subs
• Add code to Basic.subs that just calls replace if old is an UndefinedFunction.

[cklb]

I may be mistaken but only removing the change in ExprWithLimits._eval_subs still leaves broken code since it indirectly assumes that UndefinedFunction provides all properties of a Basic subclass.
(a.k.a the initial problem)
Assuming that all input is indeed a subclass of Basic may be justified but then the case handling should be pruned.

[czgdp1807]

@DarylMcCullough Thanks for the contribution. Would you still like to complete this PR? The issue is still unresolved.