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: |
Description (last modified by )
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
- Cc kcrisman added
comment:2 Changed 8 years ago by
- Milestone changed from sage-5.11 to sage-5.12
comment:3 Changed 7 years ago by
- Milestone changed from sage-6.1 to sage-6.2
comment:4 Changed 7 years ago by
- Milestone changed from sage-6.2 to sage-6.3
comment:5 Changed 7 years ago by
comment:6 Changed 7 years ago by
- Milestone changed from sage-6.3 to sage-6.4
comment:7 Changed 7 years ago by
- 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
- 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
- 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
- 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
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
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:13 Changed 4 years ago by
Actually this was a SymPy fix which should be tested in SymPy: https://github.com/sympy/sympy/blob/master/sympy/functions/combinatorial/factorials.py#L559 https://github.com/sympy/sympy/blob/master/sympy/external/tests/test_sage.py
comment:14 follow-up: ↓ 15 Changed 4 years ago by
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
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
- Resolution set to wontfix
- Status changed from positive_review to closed
Just as a point of information, nowadays
so apparently there was some extra simplification that should have happened. Doesn't mean this ticket is invalid, of course.