-
-
Notifications
You must be signed in to change notification settings - Fork 598
Add warning in LazyPowerSeriesRing.taylor() documentation #39880
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
base: develop
Are you sure you want to change the base?
Conversation
Can someone review changes, thanks |
|
hello, i think that you have only implemented the second part of the proposed solution and that is adding the warning which is definitely helpful, but you didn't implement the main fix, that is checking coefficients up to the halting precision during construction which would involve computing coefficients up to the default halting precision at initializing time and then verifying whether they can be expressed in the base ring or not, also by doing this we won't be disturbing the lazy evaluation behavior for higher precision terms. This is what i suspect is the case here, as the issue label also pointed out it's an Of course, the final implementation details could be confirmed by the maintainers, |
Documentation preview for this PR (built with commit ebaa8e7; changes) is ready! 🎉 |
@user202729 @DaveWitteMorris can you please review the PR, thanks :) |
src/sage/rings/lazy_series_ring.py
Outdated
:issue:`39838` For inputs with precision greater than the default, | ||
this does not check that the function is well-defined in the base ring:: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:issue:`39838` For inputs with precision greater than the default, | |
this does not check that the function is well-defined in the base ring:: | |
For inputs with precision greater than the default, | |
this does not check that the function is well-defined in the base ring (:issue:`39838`):: |
As is, the capitalization is incorrect since For
is not at the start of a sentence.
Also the explanation is misleading, first, the input always have infinite precision, and second this never check the function is well-defined even if a coefficient less than halting_precision does not belong to the base ring.
And this doesn't completely fix the issue, consider change the initial message to avoid https://docs.github.com/en/get-started/writing-on-github/working-with-advanced-formatting/using-keywords-in-issues-and-pull-requests
Co-authored-by: user202729 <25191436+user202729@users.noreply.github.com>
@@ -3036,6 +3036,18 @@ def taylor(self, f): | |||
For inputs as symbolic functions/expressions, this does not check | |||
that the function does not have poles at `0`. | |||
|
|||
.. WARNING:: | |||
|
|||
For inputs with precision greater than the default, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is very misleading - the default precision is infinity.
part 1 of #39838
As the title says, adds another warning in the LazyPowerSeriesRing.taylor() documentation
📝 Checklist
⌛ Dependencies