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 left and right directions to limits #9200

Closed
dcernst mannequin opened this issue Jun 10, 2010 · 18 comments
Closed

Add left and right directions to limits #9200

dcernst mannequin opened this issue Jun 10, 2010 · 18 comments

Comments

@dcernst
Copy link
Mannequin

dcernst mannequin commented Jun 10, 2010

  1. Add from_left and from_right for dir keyword.

  2. Improve error message dir keyword.

  3. Add doctests and tests.

CC: @jasongrout @rbeezer

Component: calculus

Author: Dana Ernst, Burcin Erocal

Reviewer: Rob Beezer

Merged: sage-4.6.alpha1

Issue created by migration from https://trac.sagemath.org/ticket/9200

@dcernst dcernst mannequin added this to the sage-4.6 milestone Jun 10, 2010
@dcernst dcernst mannequin added c: calculus labels Jun 10, 2010
@dcernst dcernst mannequin assigned burcin Jun 10, 2010
@dcernst
Copy link
Mannequin Author

dcernst mannequin commented Jun 10, 2010

Attachment: trac_9200-left-right-limits.patch.gz

@dcernst
Copy link
Mannequin Author

dcernst mannequin commented Jun 10, 2010

comment:1

Patch passes tests in sage/calculus. Need to test full Sage library.

@dcernst
Copy link
Mannequin Author

dcernst mannequin commented Jul 22, 2010

stand alone patch

@dcernst
Copy link
Mannequin Author

dcernst mannequin commented Jul 22, 2010

comment:2

Attachment: trac_9200-left-right-limits-v2.patch.gz

Rebased for version 4.5.1, apply only v2 patch.

@dcernst
Copy link
Mannequin Author

dcernst mannequin commented Jul 23, 2010

comment:3
  • All tests passed on version 4.5.1 (running OSX 10.6)
    • HTML & PDF reference built without problem

@dcernst dcernst mannequin added the s: needs review label Jul 23, 2010
@rbeezer
Copy link
Mannequin

rbeezer mannequin commented Jul 24, 2010

Reviewer: Rob Beezer

@rbeezer
Copy link
Mannequin

rbeezer mannequin commented Jul 24, 2010

comment:4

Applies, builds, passes all tests, and docs look fine; on 4.5.2.alpha0. So this is a positive review.

Nice job on your first contribution!

@burcin
Copy link
Contributor

burcin commented Jul 24, 2010

comment:5

Do we really need new keywords for this? And if we do, should it be from_{right,left}?

I think a user interface decision like this needs more input from other developers. I'll post to sage-devel with this question.

@dcernst
Copy link
Mannequin Author

dcernst mannequin commented Jul 25, 2010

comment:6

The idea for this came up during one of the online sessions for the MAA PREP Workshop for Sage (organized by Rob Beezer, Jason Grout, and Karl-Dieter Crisman) that I am currently enrolled in. While we were discussing calculus during one of our workshop sessions, one of the participants remarked that most students learn one-sided limits as "from the left" and "from the right." Rob, Jason, and Karl-Dieter knew that I was interested in getting involved in Sage development and remarked that I could add that this terminology to Sage if I was interested. I figured that it was an easy way to get my feet wet.

I have been using Sage for almost a year, so I would consider myself a newbie. I'm not personally attached to this patch and if others feel that it is unnecessary bloat, then my feelings won't be hurt.

@burcin
Copy link
Contributor

burcin commented Sep 8, 2010

Attachment: trac_9200-deprecation.patch.gz

add deprecation warnings

@burcin
Copy link
Contributor

burcin commented Sep 8, 2010

comment:7

This was discussed in the thread:

http://groups.google.com/group/sage-devel/t/9dd6dfe26f09be93

The resulting decision was to deprecate 'above' and 'below' and add support for '+', '-', 'right', and 'left'.

attachment: trac_9200-deprecation.patch makes the necessary changes, on top of Dana's patch.

Patches to be applied in this order:

@burcin
Copy link
Contributor

burcin commented Sep 8, 2010

Changed author from Dana Ernst to Dana Ernst, Burcin Erocal

@dcernst
Copy link
Mannequin Author

dcernst mannequin commented Sep 9, 2010

comment:8

Burcin,

Thanks for picking up the ball on this. I was planning on attacking this in a few weeks after the initial craziness of my semester settled down. Now, I get to cross something off my todo list:)

Dana

@rbeezer
Copy link
Mannequin

rbeezer mannequin commented Sep 9, 2010

Attachment: trac_9200-deprecation-v2.patch.gz

@rbeezer
Copy link
Mannequin

rbeezer mannequin commented Sep 9, 2010

comment:9

I noticed while reviewing this that there are two "TEST" headers in the docstring for limit(). So I removed the second one and uploaded a new version of the "documentation" patch - only change is the removal of the header (still has Burcin's name on it too).

I'm running tests overnight and then plan to give this a positive review.

Burcin - I'll wait for you to check-off on the one change to your patch.

Rob

@burcin
Copy link
Contributor

burcin commented Sep 9, 2010

comment:10

Thanks for taking care of the TEST headers Rob. I'm ok with your change. Looking forward to that positive review. :)

@rbeezer
Copy link
Mannequin

rbeezer mannequin commented Sep 9, 2010

comment:11

Thanks, Burcin, for the go-ahead and for prompting the discussion on this one. Builds (on 4.5.2), docs look good, passes all tests (sage -testall) and is consistent with discussion on sage-devel. Positive review.

Congrats to Dana Ernst on his first contribution. Next one will probably engender less discussion. ;-)

Release Manager

Patches to be applied in this order:

Dana Ernst is first-time contributor (for release notes).

@rbeezer rbeezer mannequin removed the s: needs review label Sep 9, 2010
@rbeezer rbeezer mannequin added the s: positive review label Sep 9, 2010
@qed777
Copy link
Mannequin

qed777 mannequin commented Sep 15, 2010

Merged: sage-4.6.alpha1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant