This is easy to fix. Basically, each assertion should be negated using the rules of http://en.wikipedia.org/wiki/De_Morgan%27s_Law , i.e., change

assert x is None and y == 0

to

if x is not None or y != 0:
    raise ReleventError("With a good error message, too.")

And also, you might as well add a raises() test to the relevant test file, if you can reproduce the exception (I have done this for the matrices, which is 846af4a by the way, and can be reviewed).

Labels: EasyToFix NeedsReview

Original comment: http://code.google.com/p/sympy/issues/detail?id=1991#c1
Original author: https://code.google.com/u/asmeurer@gmail.com/

Sometimes the assert is there because that is an assumption at that point in the code that you want the reader to be made aware of. They are like road signs that help keep you oriented...not that you couldn't look at a map and figure out where you were, but in the limited amount of lines to a screen (and finite "where am I" memory) they also provide a nanny-ish role. e.g. in the ode module there is a portion where perhaps you think that there should only be one solution and you act on that belief. I would just like to put "assert len(sol) == 1" and proceed with confidence. I maybe can't (in my limited imagination) think of a case where there would be more solutions but if, for some reason there are I want someone to make the code len(sol) > 1 compliant. "Today" I don't want to have to think up imaginative names for errors that probably won't exist. I am happy enough to know that I alerted someone -- like a zone of red on a tachometer -- that there is a pending problem.

I once looked at some personal code of Tim Peters and it was loaded with assertions. He writes some pretty slick stuff. I asked what that was for and he taught me about the assertion.

I'm not saying there isn't a place for a good error message. I am just saying that sometimes it isn't a user error as much as a logical error that the assertion protects against.

Original comment: http://code.google.com/p/sympy/issues/detail?id=1991#c2
Original author: https://code.google.com/u/117933771799683895267/

So maybe they do have their place. I just got through removing all assertions from matrix.py (see the above cited commit), because it had things like

assert self.cols == self.row

when there was even a custom NonSquareMatrixError already made. So those obviously should have been

if not self.is_square:
    raise NonSquareMatrixError("A matrix must be square to apply <whatever>.")

Here's the problem with having assertions in code like that. If you are writing general purpose code, say something that calls on an arbitrary matrix the .inv() method, which only works if the matrix is square. You then see that it sometimes fails with AssertionError, so you write your code as

try:
    M.inv()
except AssertionError:
    raise NotImplementedError

But except AssertionError will catch all AssertionError's that that code block happens to raise, and in this case, will mask them with NotImplementedError, which no longer looks like a bug. You really only wanted the try block to catch the case where the matrix inversion failed because it was not square, so it's much better to have Matrix.inv() raise NonSquareMatrixError and for your code to catch that. I remember making a change like this in solve() last year (cf781bc). I was forced to have "except AssertionError, ValueError, NotImplementedError:" in my code, where I really only wanted "except NotImplementedError:". Keeping those other ones would have masked other errors in the code that raised those exceptions. So I think you should only use assertions where the code should unequivocally stop because some assertion that has to be true for the algorithm to be correct no longer is. In other words, if the assertion does fail, it's not because of bad input, but because there is some unexpected bug in my code, which if I ever see this error I will need to go in and fix, or special case with a more descriptive exception.

So maybe the issue title should be "Don't use assert in the library code when a better exception can be used instead," i.e., ValueError, TypeError, NotImplementedError, etc. And even if the assertion does belong, it should be given an error message, which can be done by adding a string to the end of the line:

assert a == b, "We seem to have run into a problem."

Original comment: http://code.google.com/p/sympy/issues/detail?id=1991#c3
Original author: https://code.google.com/u/asmeurer@gmail.com/

**Summary:** Don't use assert in the library code when an exception would be better  
**Labels:** asmeurer  

Original comment: http://code.google.com/p/sympy/issues/detail?id=1991#c4
Original author: https://code.google.com/u/asmeurer@gmail.com/

The fix mentioned above is included in my integration3-backport branch ( https://github.com/sympy/sympy/pull/131 ).

**Status:** Started  

Original comment: http://code.google.com/p/sympy/issues/detail?id=1991#c5
Original author: https://code.google.com/u/asmeurer@gmail.com/

The patch was pushed in. I'm leaving this open because that patch only addressed the matrices code.

**Labels:** -NeedsReview -asmeurer  

Original comment: http://code.google.com/p/sympy/issues/detail?id=1991#c6
Original author: https://code.google.com/u/asmeurer@gmail.com/

I want to apply for GSOC 2014 and I am interested in solving this bug. Although I am still a little bit confused as to where assert is necessary and where it has to be replaced with a raise exception construct.

Original comment: http://code.google.com/p/sympy/issues/detail?id=1991#c7
Original author: https://code.google.com/u/100029932375768335599/

See the top of this page for a command that will show where assert is used.

Original comment: http://code.google.com/p/sympy/issues/detail?id=1991#c8
Original author: https://code.google.com/u/asmeurer@gmail.com/

Here's a useful guide on when assert should and should not be used. https://mail.python.org/pipermail/python-list/2013-November/660401.html

Original comment: http://code.google.com/p/sympy/issues/detail?id=1991#c9
Original author: https://code.google.com/u/asmeurer@gmail.com/

I addressed this issue and I have went through the entire code base and removed asserts wherever necessary.Please refer to this: #2655

Original comment: http://code.google.com/p/sympy/issues/detail?id=1991#c10
Original author: https://code.google.com/u/100029932375768335599/

PR #2655 was merged. Does any work still need to be done here?

The command from the top shows some asserts for me, although I don't know how many of them are valid. I saw at least one that isn't:

sympy/solvers/pde.py:851:        assert ValueError('Unknown strategy: %s' % strategy)

@asmeurer I have changed sympy/solvers/pde.py:851: assert ValueError('Unknown strategy: %s' % strategy) to exception
It works the same
strategy_dic={}
strategy_dic['add'] = True
strategy_dic['mul'] = False
try:
do_add = strategy_dic[strategy]
print do_add
except:
print('Unknown strategy: %s' % strategy)
(It's properly indented code just the look is different).
I have found few asserts that can be converted to exception.
sympy/solvers/diophantine.py:2660,2665,2672
Please have a look once.

Hi,

@ethandeguire and I have begun looking into this issue. We are new contributors to open source. Has any new work been done on this issue/are there any guidelines we should be aware of in fixing this? We were planning of just expanding on the work that others have done in replacing some asserts with exceptions in files where it is still necessary

Hello, I am working on this issue and I just wanted to add a note about finding these potentially unwanted assert statements:

Modified command to find offending line, and print their file and line number, excluding documentation and test files (.rst, .md)
git grep -n assert | grep -v test | grep -v .rst | grep -v .md

Or to find all files with an offending assert:
git grep -n assert | grep -v test | grep -v .rst | grep -v .md | cut -d':' -f1 | sort | uniq

I think this wasn't supposed to be automatically closed.