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

Add a evaluate keyword argument for Limit #8791

Closed
wants to merge 1 commit into from

Conversation

amitsaha
Copy link
Contributor

@amitsaha amitsaha commented Jan 9, 2015

This will make it consistent with Derivative.

This will make it consistent with Derivative.
@@ -104,7 +106,10 @@ def __new__(cls, e, z, z0, dir="+"):

obj = Expr.__new__(cls)
obj._args = (e, z, z0, dir)
return obj
if not evaluate:
Copy link
Contributor

Choose a reason for hiding this comment

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

you should respect global evaluate context.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you mean nothing should be evaluated, not just the limit?

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean you should respect global evaluate context. Please read documentation in core/evaluate.py, read other methods with evaluate kwarg.

@cbm755
Copy link
Contributor

cbm755 commented Jan 21, 2015

  • Agree with @skirpichev's comment. Here's an example from Equality:
    def __new__(cls, lhs, rhs=0, **options):
        lhs = _sympify(lhs)
        rhs = _sympify(rhs)

        evaluate = options.pop('evaluate', global_evaluate[0])
        ...
  • Also, should limit (lowercase one) have evaluate kwarg too? Then fully symmetric with how diff(..., evaluate=False) gives an unevaluated Derivative.
  • Add a test?

@czgdp1807
Copy link
Member

IMO, we don't need evaluate keyword as there is a already a function, limit which directly evaluates the limit. Probably we can close this.

@czgdp1807
Copy link
Member

Closing this PR for now. Please re-open this if needed. Thanks for the contributions.

@czgdp1807 czgdp1807 closed this Apr 20, 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.

5 participants