Repository: sympy/sympy

Issue:
UndefinedFunction.__hash__()'s use of class_key() broke existing code
Hi, I just learned that commit e210a762d (_"Allow Function('f', is_real=True) != Function('f')"_) broke my code, and I'm not sure if this is a bug in SymPy or not. The problem appears to have been caused by this change:

	class UndefinedFunction(FunctionClass):
	+    def __hash__(self):
	+        return hash((self.class_key(), frozenset(self._extra_kwargs)))

The behavior change is as follows. If I had code such as

	temp = sympy.Function(
		'temp',
		fdiff=lambda self, argindex=1: sympy.cos(self.args[argindex - 1]),
		eval=classmethod(lambda cls, t: None),
		_imp_=classmethod(lambda cls, t: sympy.sin(t))
	)
	print(sympy.diff(temp(sympy.abc.t), sympy.abc.t))

it would print

	cos(t)

under the old implementation of `__hash__`, but under the new implementation, it merely prints

	Derivative(temp(t), t)

which is undesired behavior.

It appears to me that this is a **bug** caused by the value of `_nargs` changing between successive invocations of `__hash__`, which causes the exact _same_ object to hash to two different values. However, I don't really understand the change and I'm entirely sure what the fix would be.

Would you know if this is a bug or not, and what the solution might be? Thank you.

Discussion:

Patch:
diff --git a/sympy/core/function.py b/sympy/core/function.py
index 9c4453ab50..bb488ead96 100644
--- a/sympy/core/function.py
+++ b/sympy/core/function.py
@@ -145,7 +145,6 @@ def __init__(cls, *args, **kwargs):
         # honor kwarg value or class-defined value before using
         # the number of arguments in the eval function (if present)
         nargs = kwargs.pop('nargs', cls.__dict__.get('nargs', _getnargs(cls)))
-        super(FunctionClass, cls).__init__(args, kwargs)
 
         # Canonicalize nargs here; change to set in nargs.
         if is_sequence(nargs):
@@ -162,6 +161,8 @@ def __init__(cls, *args, **kwargs):
             nargs = (as_int(nargs),)
         cls._nargs = nargs
 
+        super(FunctionClass, cls).__init__(*args, **kwargs)
+
     @property
     def __signature__(self):
         """


Test patch:
diff --git a/sympy/core/tests/test_function.py b/sympy/core/tests/test_function.py
index 6633a5ac4a..79606697b5 100644
--- a/sympy/core/tests/test_function.py
+++ b/sympy/core/tests/test_function.py
@@ -5,6 +5,7 @@
 from sympy.utilities.pytest import XFAIL, raises
 from sympy.abc import t, w, x, y, z
 from sympy.core.function import PoleError, _mexpand
+from sympy.core.sympify import sympify
 from sympy.sets.sets import FiniteSet
 from sympy.solvers.solveset import solveset
 from sympy.utilities.iterables import subsets, variations
@@ -965,3 +966,23 @@ def test_undef_fcn_float_issue_6938():
     assert not f(pi).evalf().is_number
     x = Symbol('x')
     assert not f(x).evalf(subs={x:1.2}).is_number
+
+def test_undefined_function_eval():
+    # Issue 15170. Make sure UndefinedFunction with eval defined works
+    # properly. The issue there was that the hash was determined before _nargs
+    # was set, which is included in the hash, hence changing the hash. The
+    # class is added to sympy.core.core.all_classes before the hash is
+    # changed, meaning "temp in all_classes" would fail, causing sympify(temp(t))
+    # to give a new class. We will eventually remove all_classes, but make
+    # sure this continues to work.
+
+    fdiff = lambda self, argindex=1: cos(self.args[argindex - 1])
+    eval = classmethod(lambda cls, t: None)
+    _imp_ = classmethod(lambda cls, t: sin(t))
+
+    temp = Function('temp', fdiff=fdiff, eval=eval, _imp_=_imp_)
+
+    expr = temp(t)
+    assert sympify(expr) == expr
+    assert type(sympify(expr)).fdiff.__name__ == "<lambda>"
+    assert expr.diff(t) == cos(t)