Fix problems with constructing or converting to SymPy expressions

[rwst]

In the Sympy-1.0 upgrade the AppliedUndef._sage_ method is patched away to enable the important upgrade. This function was planned to resolve part of #14723 but uncovers problems with symbolics. The following doctest in french_book fails if AppliedUndef._sage_ is defined:

sage: from sympy import Function, Symbol
sage: u = Function('u'); n = Symbol('n', integer=True)
sage: f = u(n+2)-(3/2)*u(n+1)+(1/2)*u(n)
NotImplementedError

tscrim found the minimal cases:

sage: f = function('f')
sage: x = var('x')
sage: sympy.sympify(f(x), evaluate=False)
f(x)
sage: sympy.sympify(3*f(x), evaluate=False)
AttributeError: 'Call' object has no attribute 'id'

However, we do seem to have our own troubles of constructing SymPy objects from symbolic expressions. All of these result in errors:

sage: f._sympy_()
sage: f(x)._sympy_()
sage: (f(x)+3)._sympy_()

This means that Sympy-1.0 probably only uncovered already existing problems in expression conversion. This ticket should remove the patch in build/pkgs/sympy and resolve all mentioned problems.

See #20185 for all previous discussion on this.

Depends on #23496
Depends on #22566

CC: @mforets @man74cio

Component: symbolics

Author: Marcelo Forets, Ralf Stephan

Branch/Commit: fd0db78

Reviewer: Marcelo Forets

Issue created by migration from https://trac.sagemath.org/ticket/20204

[rwst]

Description changed:

--- 
+++ 
@@ -1,4 +1,4 @@
-In the Sympy-1.0 upgrade the `UndefinedFunction._sage_` method is patched away to enable the important upgrade. This function was planned to resolve part of #14723 but uncovers missing functionality in the coercion system, and probably with symbolics. The following doctest in french_book fails if `UndefinedFunction._sage_` is defined:
+In the Sympy-1.0 upgrade the `UndefinedFunction._sage_` method is patched away to enable the important upgrade. This function was planned to resolve part of #14723 but uncovers problems with symbolics. The following doctest in french_book fails if `UndefinedFunction._sage_` is defined:
 
 ```
   sage: from sympy import Function, Symbol

[rwst]

Description changed:

--- 
+++ 
@@ -1,4 +1,4 @@
-In the Sympy-1.0 upgrade the `UndefinedFunction._sage_` method is patched away to enable the important upgrade. This function was planned to resolve part of #14723 but uncovers problems with symbolics. The following doctest in french_book fails if `UndefinedFunction._sage_` is defined:
+In the Sympy-1.0 upgrade the `AppliedUndef._sage_` method is patched away to enable the important upgrade. This function was planned to resolve part of #14723 but uncovers problems with symbolics. The following doctest in french_book fails if `AppliedUndef._sage_` is defined:
 
 ```
   sage: from sympy import Function, Symbol

[infinity0]

comment:4

Hi, any progress on this? We are currently NOT carrying Sage's sympy patch in Debian, so our Sage is running without this patch (and a few french book doctests break but we just ignore this).

In fact applying the patch, breaks some of sympy's own tests relating to Sage integration: sympy/sympy#11858 (comment)

BTW you do not need to rebuild Sympy to apply or unapply this patch, just edit /usr/lib/python2.7/dist-packages/sympy/core/function.py or wherever the file is installed to. Then restart sage (if you're testing Sage) or python (if you're testing sympy).

[mforets]

comment:5

hi, i've experimented adding a fragment of #14723:

@@ -764,6 +764,9 @@ class SympyConverter(Converter):
         if f_sympy:
             return f_sympy(*sympy.sympify(g, evaluate=False))
         else:

+            from sage.symbolic.function_factory import SymbolicFunction
+            if isinstance(ex.operator(), SymbolicFunction):
+                return sympy.Function(str(f))(*g, evaluate=False)
             raise NotImplementedError("SymPy function '%s' doesn't exist" % f)

and adding the _sympy_ method to the NewSymbolicFunction class:

@@ -67,6 +67,10 @@ def function_factory(name, nargs=0, latex_name=None, conversions=None,
             """
             return "'%s"%self.name()

+        def _sympy_(self):
+            from sympy import Function
+            return Function(self.name())

with these changes, the conversions in this ticket's description work. does this go in the good direction or not?

however, the round trip back to sage is broken:

sage: f = function('f')
sage: type(f)
<class 'sage.symbolic.function_factory.NewSymbolicFunction'>
sage: fs = f._sympy_()
sage: type(fs)
<class 'sympy.core.function.UndefinedFunction'>
sage: fs._sage_()
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-5-9fb5fbad3003> in <module>()
----> 1 fs._sage_()

TypeError: unbound method _sage_() must be called with f instance as first argument (got nothing instead)

and with a different error for a function with arguments:

sage: f = function('f')(x)
sage: type(f)
<type 'sage.symbolic.expression.Expression'>
sage: fs = f._sympy_()
sage: type(fs)
f
sage: fs._sage_()

---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-5-9fb5fbad3003> in <module>()
----> 1 fs._sage_()

/Users/forets/sage-src/sage/local/lib/python2.7/site-packages/sympy/core/function.pyc in _sage_(self)
    705         import sage.all as sage
    706         fname = self.func.__name__
--> 707         func = getattr(sage, fname)
    708         args = [arg._sage_() for arg in self.args]
    709         return func(*args)

AttributeError: 'module' object has no attribute 'f'

[rwst]

comment:6

Replying to @mforets:

with these changes, the conversions in this ticket's description work. does this go in the good direction or not?

Looks good.

[mforets]

Commit: 442d13b

[mforets]

New commits:

442d13b

check for SymbolicFunction in SympyConverter

[mforets]

Branch: u/mforets/20204

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

9f40ac9

add sympy method to NewSymbolicFunction

[sagetrac-git]

Changed commit from 442d13b to 9f40ac9

[mforets]

comment:9

for the remaining part, this is supposed to be solved here? (This ticket should remove the patch.. => to delete the file build/pkgs/sympy/patches/01_undeffun_sage.patch ??). i've played around, but i don't quite understand how it's supposed to work.. thanks.