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

Implemented _eval_nseries() for Hyper #18630

Merged
merged 6 commits into from
Feb 18, 2020
Merged

Conversation

dhruvmendiratta6
Copy link
Contributor

References to other Issues or PRs

Fixes #18193

Brief description of what is fixed or changed

In reference to Issue #18193 the function was implemented. Example of
the working code is as follows:

In [1]: from sympy import hyper

In [2]: from sympy.abc import x, y

In [3]: hyper((1, 2),(1, 2), x)._eval_nseries(x, 7, y)
Out[3]: 1 + x + x2/2 + x3/6 + x4/24 + x5/120 + x6/720 + O(x7)

In [4]: hyper((1, 2),(3, 4, 5), x)._eval_nseries(x, 7, y)
Out[4]: 1 + x/30 + x2/1200 + x3/63000 + x4/4233600 + x5/355622400 + x6/36578304000 + O(x7)

Other comments

Release Notes

  • functions
    • Added _eval_nseries() functionality to hyper

In reference to Issue sympy#18193 the function was implemented. Example of
the working code is as follows:

In [1]: from sympy import hyper

In [2]: from sympy.abc import x, y

In [3]: hyper((1, 2),(1, 2), x)._eval_nseries(x, 7, y)
Out[3]: 1 + x + x**2/2 + x**3/6 + x**4/24 + x**5/120 + x**6/720 + O(x**7)

In [4]: hyper((1, 2),(3, 4, 5), x)._eval_nseries(x, 7, y)
Out[4]: 1 + x/30 + x**2/1200 + x**3/63000 + x**4/4233600 + x**5/355622400 + x**6/36578304000 + O(x**7)
@sympy-bot
Copy link

sympy-bot commented Feb 10, 2020

Hi, I am the SymPy bot (v151). 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.6.

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.

<!-- Your title above should be a short description of what
was changed. Do not include the issue number in the title. -->

#### References to other Issues or PRs
<!-- If this pull request fixes an issue, write "Fixes #NNNN" in that exact
format, e.g. "Fixes #1234" (see
https://tinyurl.com/auto-closing for more information). Also, please
write a comment on that issue linking back to this pull request once it is
open. -->
Fixes #18193 

#### Brief description of what is fixed or changed
In reference to Issue #18193 the function was implemented. Example of
the working code is as follows:

In [1]: from sympy import hyper

In [2]: from sympy.abc import x, y

In [3]: hyper((1, 2),(1, 2), x)._eval_nseries(x, 7, y)
Out[3]: 1 + x + x**2/2 + x**3/6 + x**4/24 + x**5/120 + x**6/720 + O(x**7)

In [4]: hyper((1, 2),(3, 4, 5), x)._eval_nseries(x, 7, y)
Out[4]: 1 + x/30 + x**2/1200 + x**3/63000 + x**4/4233600 + x**5/355622400 + x**6/36578304000 + O(x**7)


#### Other comments


#### Release Notes

<!-- Write the release notes for this release below. See
https://github.com/sympy/sympy/wiki/Writing-Release-Notes for more information
on how to write release notes. The bot will check your release notes
automatically to see if they are formatted correctly. -->

<!-- BEGIN RELEASE NOTES -->
* functions
   * Added _eval_nseries() functionality to hyper
<!-- END RELEASE NOTES -->

Update

The release notes on the wiki have been updated.

@oscarbenjamin
Copy link
Contributor

This will need tests

@dhruvmendiratta6
Copy link
Contributor Author

Actually the following test is still failing. I committed it anyway as the code is working and I couldn't figure out what was going wrong.
File "c:\users\mendiratta\sympy\sympy\functions\special\tests\test_hyper.py", line 347, in test_limits O(k**6) # issue 6350 AssertionError

@jksuom
Copy link
Member

jksuom commented Feb 11, 2020

There a a couple of things that should be changed in the code. In the failing issue, the argument of the function is k**2 so that is what should appear in the series expansion. Also I think that self should be added to the super() call. Then it should be possible to fix the failing test.

bq = self.args[1]

if x0 != 0:
return super()._eval_nseries(x, n, logx)
Copy link
Member

@jksuom jksuom Feb 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return super()._eval_nseries(x, n, logx)
return super()._eval_nseries(self, x, n, logx)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure of this, though. The docs say that if the second argument of super is omitted, then the method will be unbound but I don't know what will happen if both arguments are omitted. However, there is an example that seems to indicate that the returned method will be bound to self. Perhaps that should be tested.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests seem to show that the method will be bound. So better leave this unchanged.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think what you are saying is the way it was done in python 2 and in python 3 both the arguments can be left omitted.

@jksuom
Copy link
Member

jksuom commented Feb 11, 2020

Actually, terms should be computed with expansion of arg as a series in x, but that is not needed for the existing tests.

dhruvmendiratta6 and others added 3 commits February 12, 2020 01:12
Co-Authored-By: Kalevi Suominen <jksuom@gmail.com>
Co-Authored-By: Kalevi Suominen <jksuom@gmail.com>
Co-Authored-By: Kalevi Suominen <jksuom@gmail.com>
@dhruvmendiratta6
Copy link
Contributor Author

After removing self argument from super()._eval_nseries() call which was added by mistake there are some differences while calling the _eval_nseries() of hyper and exp which are as follows:

>>> hyper((1,2),(1,2), x)._eval_nseries(y,7,None)
hyper((1, 2), (1, 2), x) + O(y**7)

>>> exp(x)._eval_nseries(y,7,None)
exp(x)

>>> exp(x**2)._eval_nseries(x,7,None)
1 + x**2 + x**4/2 + x**6/6 + x**8/24 + x**10/120 + x**12/720 + O(x**14)

>>> hyper((1,2),(1,2), x**2)._eval_nseries(x,7,None)
1 + x**2 + x**4/2 + x**6/6 + O(x**7)

is the test failing due to the difference in second output. As what I understood was that maximum power of x can be of the order of n but that is not the case with exp._eval_nseries()

Simplified expression in test_limits() to make it work with new
implementation of _eval_nseries(). Added new tests for _eval_nseries().
And renamed some variables to make readabilty of the code better
@codecov
Copy link

codecov bot commented Feb 18, 2020

Codecov Report

Merging #18630 into master will decrease coverage by 8.053%.
The diff coverage is 100%.

@@              Coverage Diff              @@
##            master    #18630       +/-   ##
=============================================
- Coverage   75.322%   67.268%   -8.054%     
=============================================
  Files          640       644        +4     
  Lines       167174    167831      +657     
  Branches     39426     39565      +139     
=============================================
- Hits        125919    112897    -13022     
- Misses       35721     49459    +13738     
+ Partials      5534      5475       -59

@jksuom
Copy link
Member

jksuom commented Feb 18, 2020

Thanks, this is ready to be merged.

@jksuom jksuom merged commit 71b4c26 into sympy:master Feb 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AttributeError: integrate(1 / (1 + x**4)**(S(1)/4), [x, 0, oo])
4 participants