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

Raise an exception when arguments to add_constraint are not admissible #9579

Closed
nathanncohen mannequin opened this issue Jul 23, 2010 · 12 comments
Closed

Raise an exception when arguments to add_constraint are not admissible #9579

nathanncohen mannequin opened this issue Jul 23, 2010 · 12 comments

Comments

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Jul 23, 2010

Because of the error reported on :

http://groups.google.com/group/sage-support/browse_thread/thread/b953192f3554337f

It may be good to save an user several hours of trouble because a wrong argument is silently accepted.

Done in this patch ! :-)

Nathann

Component: numerical

Author: Nathann Cohen

Reviewer: Harald Schilly

Merged: sage-4.5.2.alpha1

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

@nathanncohen nathanncohen mannequin added this to the sage-4.5.2 milestone Jul 23, 2010
@nathanncohen nathanncohen mannequin added the s: needs review label Jul 23, 2010
@haraldschilly
Copy link
Member

comment:2

I don't know if testing with RR is the best way to do this, but should work. What's missing is a test for max b/c you only tested for min.

@haraldschilly
Copy link
Member

test both arguments, not only one of them

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Jul 23, 2010

comment:3

Attachment: trac_9579_review.patch.gz

Replying to @haraldschilly:

I don't know if testing with RR is the best way to do this, but should work. What's missing is a test for max b/c you only tested for min.

You do not begin to know how I HATE this RR... Is there any way to check whether a value is numerical without having to import RINGS ? :-D

Even a Python method is fine !!! The most esoteric thing that could be found there is a rational number !

Nathann

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Jul 23, 2010

comment:4

Attachment: trac_9579.patch.gz

I just updated my patch so that instead of ugly "try/except", a "if" is enough, thanks to Harald's suggestion. By the way, your patch still applies on top of mine, so if you are ok with these changes, let's say this ticket is positively reviewed :-)

Nathann

@haraldschilly
Copy link
Member

comment:5

there is still this hurdle to run all tests, i'll start them right now.

@haraldschilly
Copy link
Member

Reviewer: Harald Schilly

@haraldschilly
Copy link
Member

Author: Nathan Cohen

@haraldschilly
Copy link
Member

comment:6

dear release manager, first merge trac_9579.patch and then trac_9579_review.patch

@dandrake
Copy link
Contributor

comment:7

Replying to @haraldschilly:

dear release manager, first merge trac_9579.patch and then trac_9579_review.patch

Merged in 4.5.2.alpha1.

@dandrake
Copy link
Contributor

Merged: sage-4.5.2.alpha1

@qed777
Copy link
Mannequin

qed777 mannequin commented Aug 6, 2010

Changed author from Nathan Cohen to Nathann Cohen

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Aug 6, 2010

comment:9

My 'n' !! I hadn't noticed it !! Thank you :-)

Nathann

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

3 participants