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

Incorrect Exception in dsolve() : First implementation #1803

Merged
merged 3 commits into from Feb 21, 2013

Conversation

MechCoder
Copy link
Contributor

This pull request is related to the issue http://code.google.com/p/sympy/issues/detail?id=1996

As mentioned in the above link , dsolve() returns wrong exceptions when it is not solvable , and the hint is either invalid or incorrect. I've changed it so that it raises the correct Exceptions.

from sympy import *
from sympy.abc import x
f = Function('f')
dsolve(f(x).diff(x)**2, f(x), 'separable')  #This now gives the correct ValueError
>>>  ValueError: ODE Derivative(f(x), x)**2 does not match hint separable

Also , as mentioned in the link , as needed

dsolve(f(x).diff(x)**2, f(x), 'fdsjf')  #This should give ValueError for incorrect hint
>>>  ValueError: Hint not recognized: fdsjf

Do look at the code for changes required.

@Krastanov
Copy link
Member

A rather strange error from Travis, however it does not seem to be caused by this pull request:

[..snip..]
sympy/geometry/line.py[35] ................................... [OK]
sympy/geometry/entity.py[5] ..... [OK]
================ tests finished: 2243 passed, in 168.84 seconds ================
Exception RuntimeError: 'maximum recursion depth exceeded' in <generator object _preorder_traversal at 0x3808fa0> ignored
Fatal Python error: Cannot recover from stack overflow.
Done. Build script exited with 1

@MechCoder
Copy link
Contributor Author

Yes , I noticed that in the build statement . Could you please tell what you think needs to be done , because it had passed all the tests on my laptop and I'm quite sure of the required output.

@asmeurer
Copy link
Member

Add tests (use raises).

@MechCoder
Copy link
Contributor Author

Sorry for being a noob but is the error because I haven't added tests? Could you please be a bit more clearer?

@Krastanov
Copy link
Member

The travis build error is not due to the lack of tests. The requirement for tests is a convention and it is not tested itself (that would be a bit too meta :))

For the moment just add the tests, this will cause a new travis build run and if the error persist we will discuss it again. The error itself seems unrelated to your code.

@Krastanov
Copy link
Member

More generally, there is a coverage test tool which you can use to check how much of your code is tested.

@MechCoder
Copy link
Contributor Author

I've added tests using raises as mentioned.

@asmeurer
Copy link
Member

This looks good if tests pass.

@MechCoder
Copy link
Contributor Author

Could you merge it? .It seems to have passed the Travis

asmeurer added a commit that referenced this pull request Feb 21, 2013
Incorrect Exception in dsolve() : First implementation
@asmeurer asmeurer merged commit 06c919e into sympy:master Feb 21, 2013
@glarrain
Copy link

I fear the added tests do not verify the code's behavior is correct. First of all, the messages are not checked, so the message to the user could be incorrect, with all tests passing. In fact, in line 1433

raises(ValueError, lambda: dsolve(f(x).diff(x)**2, f(x), 'seperable'))

it says 'seperable', not 'separable', thus the exception message will be "Hint not recognized: seperable" instead of the expected (I guess so) "ODE " + str(eq) + " does not match hint separable".

@MechCoder MechCoder deleted the 1996_issue branch February 21, 2013 20:20
@asmeurer
Copy link
Member

Oh, you're right. That should be fixed.

@asmeurer
Copy link
Member

This was so trivial that I just pushed a fix in.

@asmeurer
Copy link
Member

asmeurer commented Mar 2, 2013

SymPy Bot Summary: ❌ Could not fetch the branch Manoj-Kumar-S/1996_issue.
@manoj-kumar-s: Please make sure that Manoj-Kumar-S/1996_issue has been pushed to GitHub and run the sympy-bot tests again.

@MechCoder MechCoder restored the 1996_issue branch March 2, 2013 16:05
@MechCoder MechCoder deleted the 1996_issue branch March 8, 2013 13:05
goodok pushed a commit to goodok/sympy that referenced this pull request Apr 6, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants