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

do not evaluate unnecessarily #35291

Merged
merged 2 commits into from
Apr 1, 2023

Conversation

mantepse
Copy link
Collaborator

@mantepse mantepse commented Mar 15, 2023

📚 Description

Currently, Stream_cauchy_compose unnecessarily evaluates its right argument g (implicitly), even if the corresponding coefficient of the left argument f is zero. This breaks various recursive definitions.

Fixes #35261.

📝 Checklist

  • I have made sure that the title is self-explanatory and the description concisely explains the PR.
  • I have linked an issue or discussion.
  • I have created tests covering the changes.
  • I have updated the documentation accordingly.

⌛ Dependencies

Copy link
Collaborator

@tscrim tscrim left a comment

Choose a reason for hiding this comment

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

Thank you for pushing the fix. Sorry I was slow at doing it.

@vbraun vbraun merged commit 70388e8 into sagemath:develop Apr 1, 2023
vbraun pushed a commit that referenced this pull request Apr 1, 2023
    
<!-- ^^^^^
Please provide a concise, informative and self-explanatory title.
Don't put issue numbers in there, do this in the PR body below.
For example, instead of "Fixes #1234" use "Introduce new method to
calculate 1+1"
-->
### 📚 Description

This address the arity problem noted in #35261. It also removes a `TODO`
in a `revert()` implementation by making the behavior the same across
different implementations. Warnings are added for the user for the
assumptions made by the code.

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->

- [x] I have made sure that the title is self-explanatory and the
description concisely explains the PR.
- [x] I have linked an issue or discussion.
- [x] I have created tests covering the changes.
- [x] I have updated the documentation accordingly.

### ⌛ Dependencies
<!-- List all open pull requests that this PR logically depends on -->

- #35127 Avoiding a long test.
- #35254 Avoiding merge conflicts.
- #35291 Making sure these changes work together.
- #35265 For some slight simplification of the new stream's
`__getitem__`.
    
URL: #35293
Reported by: Travis Scrimshaw
Reviewer(s): Martin Rubey, Travis Scrimshaw
@mantepse mantepse deleted the be_more_lazy_in_compose branch April 1, 2023 17:42
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.

bug in revert method for lazy power series
4 participants