Opened 8 years ago

Closed 4 years ago

#14446 closed defect (wontfix)

missing Sage interface for SymPy's RisingFactorial

Reported by: zimmerma Owned by: was
Priority: minor Milestone: sage-duplicate/invalid/wontfix
Component: interfaces Keywords:
Cc: kcrisman Merged in:
Authors: Reviewers: Paul Zimmermann
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by rws)

Minimal test case:

sage: import sympy
sage: SR(sympy.RisingFactorial(x,x))
AttributeError: 'RisingFactorial' object has no attribute '_sage_'
sage: SR(sympy.FallingFactorial(x,x))
AttributeError: 'FallingFactorial' object has no attribute '_sage_'

Original ticket description:

This is related to #14437:

sage: from sympy import Function, Symbol
sage: u = Function('u')
sage: n = Symbol('n', integer=True)
sage: from sympy import rsolve
sage: f = u(n+2) - u(n+1) + u(n)/4
sage: rsolve(f,u(n))
2**(-n)*C0*RisingFactorial(C0/C1 + 1, n)/RisingFactorial(C0/C1, n)

The result returned by rsolve is a SymPy? object:

sage: s = rsolve(f,u(n))
sage: type(s)
<class 'sympy.core.mul.Mul'>

Ideally, it should be automatically converted to Sage.

However, if we try to convert it manually, we get:

sage: s._sage_()
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)

/users/caramel/zimmerma/svn/sagebook/tex/<ipython console> in <module>()

/usr/local/sage-5.1-linux-64bit-ubuntu_12.04_lts-x86_64-Linux/local/lib/python2.7/site-packages/sympy/core/mul.py in _sage_(self)
   1192         s = 1
   1193         for x in self.args:
-> 1194             s *= x._sage_()
   1195         return s
   1196 

/usr/local/sage-5.1-linux-64bit-ubuntu_12.04_lts-x86_64-Linux/local/lib/python2.7/site-packages/sympy/core/power.py in _sage_(self)
    846 
    847     def _sage_(self):
--> 848         return self.args[0]._sage_()**self.args[1]._sage_()
    849 
    850 from add import Add

AttributeError: 'RisingFactorial' object has no attribute '_sage_'

Paul

Change History (16)

comment:1 Changed 8 years ago by kcrisman

  • Cc kcrisman added

comment:2 Changed 8 years ago by jdemeyer

  • Milestone changed from sage-5.11 to sage-5.12

comment:3 Changed 7 years ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:4 Changed 7 years ago by vbraun_spam

  • Milestone changed from sage-6.2 to sage-6.3

comment:5 Changed 7 years ago by kcrisman

Just as a point of information, nowadays

sage: rsolve(f,u(n))
2**(-n)*(C0 + C1*n)

so apparently there was some extra simplification that should have happened. Doesn't mean this ticket is invalid, of course.

comment:6 Changed 7 years ago by vbraun_spam

  • Milestone changed from sage-6.3 to sage-6.4

comment:7 Changed 7 years ago by rws

  • Component changed from calculus to interfaces
  • Owner changed from burcin to was
  • Summary changed from missing Sage equivalent for RisingFactorial to missing Sage interface for SymPy's RisingFactorial

comment:8 Changed 7 years ago by rws

  • Description modified (diff)

This is now fixed in the proposed sympy pull request https://github.com/sympy/sympy/pull/8592 but we will upload a branch with the specific sympy patch next.

comment:9 Changed 4 years ago by rws

  • Milestone changed from sage-6.4 to sage-duplicate/invalid/wontfix
  • Status changed from new to needs_review

This was resolved long ago by the latest SymPy update.

comment:10 Changed 4 years ago by zimmerma

  • Reviewers set to Paul Zimmermann
  • Status changed from needs_review to positive_review

fixed indeed in Sage 8.0 (or before).

comment:11 Changed 4 years ago by kcrisman

I've noticed a few recent closed tickets which were apparently resolved -- but no one mentioned doctesting these resolutions. We should do that to avoid regressions. (Unless they are already tested, of course.)

comment:12 Changed 4 years ago by zimmerma

I tried adding a doctest with those commands:

$ git clone git://git.sagemath.org/sage.git
$ cd sage
$ git branch u/zimmerma/14446
$ git co u/zimmerma/14446
$ patch -p1 -i /tmp/patch
$ git commit -a
$ git push --set-upstream origin u/zimmerma/14446

but it failed with:

$ git push --set-upstream origin u/zimmerma/14446
fatal: remote error: access denied or repository not exported: /sage.git

Am I doing something wrong?

For the record:

$ cat /tmp/patch
commit ef0235ff466d40c787681e3e483342e091247392
Author: Paul Zimmermann <Paul.Zimmermann@inria.fr>
Date:   Tue Oct 3 13:03:18 2017 +0200

    added doctest for #14446

diff --git a/src/sage/calculus/test_sympy.py b/src/sage/calculus/test_sympy.py
index 996e417..15b1d7e 100644
--- a/src/sage/calculus/test_sympy.py
+++ b/src/sage/calculus/test_sympy.py
@@ -196,4 +196,15 @@ This was fixed in Sympy, see :trac:`14437`::
     sage: rsolve(f,u(n))
     2**(-n)*(C0 + C1*n)
 
+This was fixed in Sympy, see :trac:`14446`::
+
+    sage: import sympy
+    sage: SR(sympy.RisingFactorial(x,x))
+    gamma(2*x)/gamma(x)
+    sage: SR(sympy.RisingFactorial(x,3))
+    (x + 2)*(x + 1)*x
+    sage: SR(sympy.FallingFactorial(x,x))
+    gamma(x + 1)
+    sage: SR(sympy.FallingFactorial(x,3))
+    (x - 1)*(x - 2)*x
 """

comment:14 follow-up: Changed 4 years ago by kcrisman

That is very interesting. Is it possible to have this sympy file doctested whenever Sage runs doctests? (I don't know how many people would run the package doctests, and in any case this probably isn't automatically tested as easily within sympy itself.) That would be very helpful.

comment:15 in reply to: ↑ 14 Changed 4 years ago by rws

Replying to kcrisman:

That is very interesting. Is it possible to have this sympy file doctested whenever Sage runs doctests?

Just copy it at SymPy installation time into src/sage/tests.

this probably isn't automatically tested as easily within sympy itself

You're right. Their Travis doesn't have a Sage installation AFAIK.

That would be very helpful.

So I opened #23960.

comment:16 Changed 4 years ago by embray

  • Resolution set to wontfix
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.