Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

missing Sage interface for SymPy's RisingFactorial #14446

Closed
zimmermann6 opened this issue Apr 11, 2013 · 11 comments
Closed

missing Sage interface for SymPy's RisingFactorial #14446

zimmermann6 opened this issue Apr 11, 2013 · 11 comments

Comments

@zimmermann6
Copy link

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

CC: @kcrisman

Component: interfaces

Reviewer: Paul Zimmermann

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

@zimmermann6 zimmermann6 added this to the sage-5.11 milestone Apr 11, 2013
@jdemeyer jdemeyer modified the milestones: sage-5.11, sage-5.12 Aug 13, 2013
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.1, sage-6.2 Jan 30, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.2, sage-6.3 May 6, 2014
@kcrisman
Copy link
Member

comment:5

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.

@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.3, sage-6.4 Aug 10, 2014
@rwst rwst changed the title missing Sage equivalent for RisingFactorial missing Sage interface for SymPy's RisingFactorial Dec 13, 2014
@rwst rwst assigned williamstein and unassigned burcin Dec 13, 2014
@rwst

This comment has been minimized.

@rwst
Copy link

rwst commented Dec 14, 2014

comment:8

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

@rwst
Copy link

rwst commented Oct 3, 2017

comment:9

This was resolved long ago by the latest SymPy update.

@rwst rwst removed this from the sage-6.4 milestone Oct 3, 2017
@zimmermann6
Copy link
Author

Reviewer: Paul Zimmermann

@zimmermann6
Copy link
Author

comment:10

fixed indeed in Sage 8.0 (or before).

@kcrisman
Copy link
Member

kcrisman commented Oct 3, 2017

comment:11

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.)

@zimmermann6
Copy link
Author

comment:12

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
 """

@rwst
Copy link

rwst commented Oct 3, 2017

@kcrisman
Copy link
Member

kcrisman commented Oct 3, 2017

comment:14

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.

@rwst
Copy link

rwst commented Oct 3, 2017

comment:15

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants