Returns valid result for functions like sin(f(x)) #15516
Conversation
|
✅ Hi, I am the SymPy bot (v134). I'm here to help you write a release notes entry. Please read the guide on how to write release notes. Your release notes are in good order. Here is what the release notes will look like:
This will be added to https://github.com/sympy/sympy/wiki/Release-Notes-for-1.4. Note: This comment will be updated with the latest check if you edit the pull request. You need to reload the page to see it. Click here to see the pull request description that was parsed. |
sympy/functions/special/bessel.py
Outdated
| def taylor_term(n, x, *previous_terms): | ||
| def taylor_term(self, n, x, *previous_terms): | ||
| if self.args[0] != x: | ||
| return super(sin, self).taylor_term(n, x, *previous_terms) |
There was a problem hiding this comment.
This is in Airy function class. Why super(sin, self)? And why the change in Airy but not any other functions?
There was a problem hiding this comment.
Oh! That was a silly of me, I thought I changed the Sin class
| @@ -1124,9 +1124,11 @@ def fdiff(self, argindex=1): | |||
| else: | |||
| raise ArgumentIndexError(self, argindex) | |||
|
|
|||
| @staticmethod | |||
There was a problem hiding this comment.
I'm not clear why talor_term is a staticmethod, but does this break the API?
There was a problem hiding this comment.
@asmeurer being a staticmethod implies sin(2*x).taylor_term(1, x) and sin(x).taylor_term(1, x) will return the same thing, which doesn't seem right. Of course, removing the decorator is not the only thing to do here.
There was a problem hiding this comment.
Yes, I don't understand why it is done that way, but I noticed that all the other taylor_term methods in SymPy are staticmethods.
There was a problem hiding this comment.
This is the only place I found in the code where taylor_term is called on a class, and it can just be replaced with self.taylor_term:
I think we can most likely remove the static method. If someone needs to generate the taylor term for specific class they can create an instance with a dummy variable. It's a (minor) backwards compatibility break so if we do refactor this we should be sure to mention it in the relevant section of the release notes.
|
I just made some changes. Please check it out. |
|
@m-agboola Are you still working on this because i want to work on this issue. |
|
Yes @RituRajSingh878. I'm updating my pull request very soon |
|
Are you still working on this? |
oscarbenjamin
added
the
PR: author's turn
|
@m-agboola Are you still working on is? Please resolve the conflicts. |
|
|
||
| cdef double z = 0 | ||
| test(x, y, &z) | ||
| return z |
|
I think you have deleted, |
|
Closing this in favour of #15603 |
Returns valid result for functions like sin(f(x))
(all as suggested by @normalhuman)
Fixes issue #15511
Earlier,
In [12]: A = sin(x**2 + 2*x) In [13]: A.taylor_term(4, x) Out[13]: 0Now
In [12]: A = sin(x**2 + 2*x) In [13]: A.taylor_term(4, x) Out[13]: -2x**4Release Notes