Skip to content

introduce limit(expr, v, a) syntax #38780

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.

Sign up for GitHub

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

Jump to bottom
Draft
wants to merge 1 commit into
base: develop
Choose a base branch
from
Draft

introduce limit(expr, v, a) syntax #38780

wants to merge 1 commit into from

Conversation

nbruin
Copy link
Contributor

@nbruin nbruin commented Oct 7, 2024

following up on discussion https://groups.google.com/g/sage-support/c/o3r9bObqz3s, we introduce the notation

limit(expr, v, a)

here to get a more direct interface with the internals. The current syntax only accepts limit(expr, v=a) which is cute, but it requires the variable to be denoted by a string. This can be problematic when, for instance, you want to take a limit with respect to X[0] where X=var('x',n=3), in which case the string description of the variable is not directly available.

📝 Checklist

  • The title is concise and informative.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.
  • I have created tests covering the changes.
  • I have updated the documentation and checked the documentation preview.

⌛ Dependencies

@nbruin
Copy link
Contributor Author

nbruin commented Oct 7, 2024

small incompatibility:

limit(1/x,"+",x=0)

used to work (because dir would capture the first positional argument) and it doesn't anymore now. It could be special-cased, but the recommended way to call it was limit(1/x,x=0,dir="+"), which still works.

I guess limit(expr, v=a) used to work as well and won't anymore because of the clash. We could avoid optional variable clashes by processing *args to only capture explicitly positional arguments.

@nbruin nbruin marked this pull request as draft October 7, 2024 01:04
@nbruin
Copy link
Contributor Author

nbruin commented Oct 7, 2024

To anyone who has inspiration: Please finish this ticket. I just made this as a place holder and am not particularly interested in pushing this feature to completion.

@github-actions GitHub Actions
Copy link

github-actions bot commented Oct 7, 2024

Documentation preview for this PR (built with commit cda5d2a; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

@vincentmacri
Copy link
Contributor

Related issue: #38761.

@EigenVector22
Copy link
Contributor

EigenVector22 commented Mar 4, 2025 •
edited
Loading

hi, I would like to work on this issue.
I understand the issue is about adding a new limit(expr, v, a) syntax, I'll add doctests for this new functionality. @vincentmacri @fchapoton can you please assign me this

@vincentmacri
Copy link
Contributor

@EigenVector22 good luck! Since it looks like you're a new contributor, here a few useful links:

and the standards we expect a PR to meet before getting merged as well as our review process are explained here: https://doc.sagemath.org/html/en/developer/review.html

@EigenVector22 EigenVector22 mentioned this pull request Mar 28, 2025
Merged
5 tasks
vbraun pushed a commit to vbraun/sage that referenced this pull request May 6, 2025
1273e57 
sagemathgh-39812: Adds limit(expr, v, a) syntax
    
Implemented the limit(expr, variable, value) positional syntax to allow
limits with respect to indexed variables or other variables not usable
as keyword arguments.

Also, Updated the documentation and added doctests for the new syntax
and associated error handling. Ensuring code coverage for the new
argument parsing.

Fixes sagemath#38761  by allowing limits to be taken with respect to indexed
variables like x[0] or other symbolic expressions not usable as keyword
arguments.

While testing, the tests passed except the optional fricas ones that
were failing before too.

<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->



### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [x] I have created tests covering the changes.
- [ ] I have updated the documentation and checked the documentation
preview.

### ⌛ Dependencies

Got the idea and direction of fixing the issue and the link to the
relevant conversations in the given PR: sagemath#38780
<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#39812
Reported by: Ashutosh Rajora
Reviewer(s): Ashutosh Rajora, Dima Pasechnik, nbruin, Vincent Macri
vbraun pushed a commit to vbraun/sage that referenced this pull request May 9, 2025
4dfd788 
sagemathgh-39812: Adds limit(expr, v, a) syntax
    
Implemented the limit(expr, variable, value) positional syntax to allow
limits with respect to indexed variables or other variables not usable
as keyword arguments.

Also, Updated the documentation and added doctests for the new syntax
and associated error handling. Ensuring code coverage for the new
argument parsing.

Fixes sagemath#38761  by allowing limits to be taken with respect to indexed
variables like x[0] or other symbolic expressions not usable as keyword
arguments.

While testing, the tests passed except the optional fricas ones that
were failing before too.

<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->



### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [x] I have created tests covering the changes.
- [ ] I have updated the documentation and checked the documentation
preview.

### ⌛ Dependencies

Got the idea and direction of fixing the issue and the link to the
relevant conversations in the given PR: sagemath#38780
<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#39812
Reported by: Ashutosh Rajora
Reviewer(s): Ashutosh Rajora, Dima Pasechnik, nbruin, Vincent Macri
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@EigenVector22 EigenVector22

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@nbruin @vincentmacri @EigenVector22