Which exception should I raise on bad/illegal argument combinations in Python?

Question

I was wondering about the best practices for indicating invalid argument combinations in Python. I've come across a few situations where you have a function like so:

def import_to_orm(name, save=False, recurse=False):
    """
    :param name: Name of some external entity to import.
    :param save: Save the ORM object before returning.
    :param recurse: Attempt to import associated objects as well. Because you
        need the original object to have a key to relate to, save must be
        `True` for recurse to be `True`.
    :raise BadValueError: If `recurse and not save`.
    :return: The ORM object.
    """
    pass

The only annoyance with this is that every package has its own, usually slightly differing BadValueError. I know that in Java there exists java.lang.IllegalArgumentException -- is it well understood that everybody will be creating their own BadValueErrors in Python or is there another, preferred method?

Solution 1

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?

Solution 2

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.

Solution 3

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.

Solution 4

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

Solution 5

I've mostly just seen the builtin ValueError used in this situation.

View more solutions

Comments

[sedavidw]

Compare google.com/codesearch?q=lang:python+class\+\wError(([^E]‌​\w*|E[^x]\w)): with google.com/codesearch?q=lang:python+class\+\w*Error(Exce‌​ption):

[cdleary]

@dbr: Yeah, I think they mean "(built-in operation or function)", not "(built-in operation) or function". I would think they'd have contrasted it by saying "user defined" in the second case.

[cdleary]

@MizardX: That is interesting, as is google.com/codesearch?q=lang%3Apython+class\+IllegalArgument‌​Exception -- they split between inheritance from ValueError, Exception, and BaseException.

[Ignacio Vazquez-Abrams]

That blurb simply means that built-ins raise it, and not that only built-ins can raise it. It would not be entirely appropriate in this instance for the Python documentation to talk about what external libraries raise.

[James Bennett]

Every piece of Python software I've ever seen has used ValueError for this sort of thing, so I think you're trying to read too much into the documentation.

[Kevin Little]

> "so why not use that?" - Specificity. Perhaps I want to catch at some outer layer "MyValueError", but not any/all "ValueError".

[cdleary]

@James Bennett: We cited several projects in the above Google code searches that do not use ValueError directly, so there at least seems to be a need for clarification.

[cdleary]

Yeah, so part of the question of specificity is where else ValueError is raised. If the callee function likes your arguments but calls math.sqrt(-1) internally, a caller may be catching ValueError expect that its arguments were inappropriate. Maybe you just check the message in this case...

[Hamish Grubijan]

Stop writing classes and custom exceptions - pyvideo.org/video/880/stop-writing-classes

[Eponymous]

As stated, the documentation is ambiguous. It should have been written as either "Raised when a built-in operation or built-in function receives" or as "Raised when a function or built-in operation receives". Of course, whatever the original intent, current practice has trumped it (as @dbr points out). So it should be rewritten as the second variant.

[ereOn]

I'm not sure that argument holds: if someone is calling math.sqrt(-1), that's a programming error that needs to be fixed anyway. ValueError is not intended to be caught in normal program execution or it would derive from RuntimeError.

[João dos Reis]

@HamishGrubijan that video is terrible. When anyone suggested a good use of a class, he just bleated "Don't use classes." Brilliant. Classes are good. But don't take my word for it.

[RayLuo]

@RobertGrant No, you don't get it. That video is not really about literally "don't use classes". It is about don't over-complicate things.

[Paul Rooney]

This should be a comment not an answer.

[João dos Reis]

@RayLuo you may have sanity-checked what the video's saying and converted it into a palateable, sensible alternative message, but that is what the video says, and it's what someone who doesn't have a lot of experience and common sense will come away with.

[Erik Aronesty]

If the error is on the NUMBER of arguments, for a function with a variable number of arguments ... for example a function where the arguments must be an even number of arguments, then you should raise a TypeError, to be consistent. And don't make your own class unless a) you have a use case or b) you are exporting the library to be used by others. Premature functionality is the death of code.

[Samuel]

@RobertGrant As I understand it, he is talking against bad usage of classes, not against any usage of classes (see from minute 12:48). And he did say that there is a risk to over simplify when answering the first question. So the key is to find a balance, no?

[João dos Reis]

@SamuelSantana as I said, any time anyone put their hand up and said "what about X?" where X was a good idea, he just said, "don't make another class." Pretty clear. I agree the key is balance; the problem is that's just too vague to actually live by :-)

[BobHy]

Nothing about the cited doc prevents you from raising ValueError in user-written code. And widespread usage in Python community is to use in their own code.

[BobHy]

Not quite what OP wanted to flag. Values of arguments might be all right, but combinations of arguments are the problem. So IllegalArgument isn't the right word, BadArguments is more like it. I think so much of that idea that I'll propose it as a different answer.

[cowbert]

I'd raise KeyError for keyword not found (since a missing explicit keyword is semantically identical to a **kwargs dict that is missing that key).

[user3504575]

Nothing surprises me but I agree 100% that TypeError is the correct exception if the type is wrong on some of the arguments passed into the function. A ValueError would be suitable if the variables are of correct type but their contents and values does not make sense.

[Nobody]

I think this is probably for missing or uncalled for arguments, while the question is about arguments that are given correctly, but are incorrect on a higher abstraction level involving the value of the given argument. But as I was actually looking for the former, so have an upvote anyway.

[goodmami]

As @user3504575 and @Nobody said, TypeError is used if the arguments don't match the function signature (wrong number of positional arguments, keyword arguments with the wrong name, wrong type of argument), but a ValueError is used when the function call matches the signature but the argument values are invalid (e.g., calling int('a')). source

[J Bones]

As the OP's question referred to "invalid argument combinations", it seems a TypeError would be appropriate as this would be a case where the function signature is essentially wrong for the passed arguments.

[goodmami]

Your example calls sum() with no arguments, which is a TypeError, but the OP was concerned with "illegal" combinations of argument values when the argument types are correct. In this case, both save and recurse are bools, but if recurse is True then save should not be False. This is a ValueError. I agree that some interpretation of the question's title would be answered by TypeError, but not for the example that's presented.

[Siu Ching Pong -Asuka Kenji-]

I think this is the most accurate answer. This is obviously underrated (7 votes so far including mine).

[vlizana]

Would an assertion also be acceptable in this case or is there a specific reason to use ValueError instead?

[Gloweye]

@vlizana This case is a ValueError because it's the correct type, but an invalid value. Assertions are a bad idea in this case because they get optimized out; but that goes out of topic for this question. See stackoverflow.com/a/5142453/4331885 for more details.