Which exception should I raise on bad/illegal argument combinations in Python?
It depends on what the problem with the arguments is.
If the argument has the wrong type, raise a TypeError. For example, when you get a string instead of one of those Booleans.
if not isinstance(save, bool):
raise TypeError(f"Argument save must be of type bool, not {type(save)}")
Note, however, that in Python we rarely make any checks like this. If the argument really is invalid, some deeper function will probably do the complaining for us. And if we only check the boolean value, perhaps some code user will later just feed it a string knowing that non-empty strings are always True. It might save him a cast.
If the arguments have invalid values, raise ValueError. This seems more appropriate in your case:
if recurse and not save:
raise ValueError("If recurse is True, save should be True too")
Or in this specific case, have a True value of recurse imply a True value of save. Since I would consider this a recovery from an error, you might also want to complain in the log.
if recurse and not save:
logging.warning("Bad arguments in import_to_orm() - if recurse is True, so should save be")
save = True
I think the best way to handle this is the way python itself handles it. Python raises a TypeError. For example:
$ python -c 'print(sum())'
Traceback (most recent call last):
File "<string>", line 1, in <module>
TypeError: sum expected at least 1 arguments, got 0
Our junior dev just found this page in a google search for "python exception wrong arguments" and I'm surprised that the obvious (to me) answer wasn't ever suggested in the decade since this question was asked.
I would inherit from ValueError
class IllegalArgumentError(ValueError):
pass
It is sometimes better to create your own exceptions, but inherit from a built-in one, which is as close to what you want as possible.
If you need to catch that specific error, it is helpful to have a name.
I would just raise ValueError, unless you need a more specific exception..
def import_to_orm(name, save=False, recurse=False):
if recurse and not save:
raise ValueError("save must be True if recurse is True")
There's really no point in doing class BadValueError(ValueError):pass
- your custom class is identical in use to ValueError, so why not use that?