How should I terminate `if ... elif ... elif...` when all options are exhausted?

Python Help
21

I’ve been watching this discussion with some bemusement.

Let me say first up that I’m a big fan of the trailing “raise exception
if a case is not handled” approach. I’ll try to explain why below, with
a real world example from my own code.

But first, Elis’ statement “From a technical standpoint, ValueError
will never be raised” is, I think, incorrect. Here’s his example code
and commentry:

def do_action(value: float, option: Options):
   if option is Options.IDENTITY:
       new_value = value
   elif option is Options.DOUBLE:
       new_value = value * 2
   elif options is Options.SQUARE:
       new_value = value**2
   else:
       raise ValueError(f'Unhandled option {option}')
   return new_value

From a technical standpoint, ValueError will never be raised.

Why? What if Options grew a new enumeration value? What if
do_action is called by code with an outright incorrect value i.e. not
in Options’ allowed values?

I think his scenario is a fully typechecked codebase that never reached
deployment without passing the type checks. But I might release fully
checked code into the wild and an untypechecked codebase might call it
incorrectly.

And if it does get raised, it’s the incorrect exception to occur.
TypeError would be the suitable exception to raise.

Maybe. But let me offer a counter example. Suppose Options grows a new
enumeration value, eg CUBE? The code above is valid, a valid caller
might call it with Options.CUBE, a type checker will be happy, and the
function would behave incorrectly.

My common case with this scenario is command line option handling. I’ve
got a command line base class which can require an apply_opt method.
Here’s a real example:

def apply_opt(self, opt, val):
     ''' Handle an individual global command line option.
     '''
     options = self.options
     if opt == '-d':
       if not isdirpath(val):
         raise GetoptError("not a directory: %r" % (val,))
       options.spdpath = val
     elif opt == '-n':
       options.doit = False
     else:
       raise RuntimeError("unhandled pre-option")

This code catches the gap between the allowed options (constrained by a
getopt specification earlier) and my implementation of those options.

(Let’s not digress into argparse vs getopt; I hate argparse and
getopt does what I want.)

I’m not raising ValueError here - the opt should be valid here
because the parse phase winnows out unspecified options; instead I’m
raising RuntimeError because this is a bug in my code - a newly
specified option with no implementation of it.

But one can also invent scenarios where ValueError is a valid
exception to raise, for example (contrived):

 def do_action(value: float, option: Options):
     if option is Options.IDENTITY:
         new_value = value
     elif option is Options.DOUBLE:
         new_value = value * 2
     elif options is Options.SQUARE:
         new_value = value**2
     elif options is Options.SQUARE_ROOT and value >= 0:
         new_value = sqrt(value)
     else:
         raise ValueError(f'Unhandled option {option}')
     return new_value

Ths isn’t how I’d code this myself (I’d whinge explicitly about the
negative value) but this will cause a ValueError in a suitable
situation.

Personally, I think all code of the form “pick amongst an explicitly
enumerated set of choices” wants a catchall at the bottom for a choice
not handled. What that catchall does depends on the function, but not
having it lends the code to bugs. Fail early! If your code it truly
robust the catchall will never fire. But all it takes is a small change
for it to become reachable.

Arguments against remind me of a code review I once had where the
reviewer wanted me to drop just such a catchall (which just logged a
warning) on the basis that it wouldn’t happen in the real world. But
they also argued that the warning would bloat the logs. These seemed
inconsistent to me.

Provide a catchall, and raise an exception on unexpected circumstances.
It is defensive and Pythonic.

Cheers,
Cameron Simpson cs@cskk.id.au

3 Likes
22

Since you’re utilizing an enumerated data structure, there’s a mathematical assurance that its size won’t expand. If it does, it would no longer qualify as an enumeration. This is precisely why I questioned, ‘Why are you using Enum?’

An enumeration is a complete, ordered listing of all the items in a collection.

If you possess an Enum representing weekdays and you employ it for a function exclusively designed for weekends, it would be technically inaccurate. This is because you’re utilizing the wrong Enum for the purpose.

Of course, you can reorganize the code. This would involve making sure that all constants in the “Options” Enum are used within the “do_action” function. You can easily verify this with a test case. But remember, this validation shouldn’t be a part of the function itself.

Furthermore, you can easily enforce typing using assert. Lastly, they are already using a function to validate all these options.

23

Yes, this is at the core of what I’m getting at with this question. Basically is it good practice to have a catchall in this sort of scenario. I’m not as interested in how exactly the catchall is realized (early returns, else, match, dispatcher, etc.), just the question of whether it should be present.

@elis.byberi does bring up the point that in the example I posted, the input to my get_round_digits is already validated. In fact get_round_digits is indeed a back-end function and won’t be user facing*. It could be renamed _get_round_digits. So in this case the else clause is only

  • protecting devs from seeing pycharm warnings
  • protecting devs from making a mistake such as adding a new option to the enum but forgetting to add it to the handled cases for this function
  • acting as a message to readers saying explicitly “all of the above is all I handle”.

Otherwise it will never be hit. So from that perspective it feels like a waste of code. Especially if a test has to get written for it also.

So is it still worth it to have the “catchall” even for a back-end function that is only called by a caller which itself already does validation on options it’s passing around?

For me, it feels like the pros outweigh the cons, so that for me this is a good practice.

*But maybe one day it will change and it will become public.

1 Like
24

This is Python. There is no compiler verifying the types of arguments passed to a function. If a function is at the interface boundary of a library or application, as a user I can pass whatever I please as a value and if the thing I pass happens to have the object attributes the function uses then it will work.

  1. This is one of an overloaded set of definitions for “enumeration” as it gets used in programming languages. “Enum” is used in several mainstream languages to refer to features that more resemble sum types [1]
  2. In cases like the above, I myself as the API designer control that the items in the enumeration are. They’re not defined by some universal physical law.

Equivalently, you can use a static type checking tool to verify exactly the same thing, without writing a test case. This aligns with a general tradeoff between unit testing and gradual type checking in dynamic programming languages. Dynamic languages tend to require more tests to verify correctness, whereas static type systems implicitly cover a subset of the conditions you’d otherwise need to test for. There isn’t a one-size-fits-all rule for this.

This is not what assertions are for. The assert statement is a self-defense measure for program state invariants. If you are receiving arguments across an API boundary, assert is not an appropriate mechanism for type validation, for reasons others have already mentioned - not in Python and not in most other languages that include assert with similar semantics. [2]

On top of all the other pros you mentioned, code is read more often then written and clarity trumps concision. Having the ‘else’ case is explicit documentation of intent: that the programmer considered condition exhaustiveness and defined it as a precondition failure. And from there it’s a matter of failing early vs late, and raising an exception surfaces the precondition failure as early as possible.

The tradeoffs are all different in a compiled language that will validate this stuff at build time, but I think the trailing catch all is a nice hybrid of typed and dynamic Python - it helps the author(s), it helps the type checker, and it helps with runtime errors.

  1. Tagged union - Wikipedia ↩︎

  2. The statements being either disabled or compiled out in non-debug build/run configurations ↩︎

2 Likes
25

This differs from Python Enum definition, but you have the freedom to misuse Enum for implementing a tagged union.

An enumeration:

  • is a set of symbolic names (members) bound to unique values
  • can be iterated over to return its canonical (i.e. non-alias) members in definition order
  • uses call syntax to return members by value
  • uses index syntax to return members by name
  • The assert statement should only be used during development (although running code without the -O option will have no effect if the code is correct). It was a suggestion to enforce typing, not for validating user input. Please, let’s not deviate from the usage of assert.
  • You must validate user input. You cannot rely on a static type checker alone to validate user input.
  • Static type checkers are not meant to replace testing procedures. You should test the code after each modification. Using assert_never doesn’t ensure the correctness of the function code; it simply indicates that there are unhandled options. If you have thoroughly tested the code, including the user input validation code, there’s no necessity to incorporate assert_never in the production code. Put differently, it would be more advantageous to use the conventional ‘assert’ for enforcing typing. This approach would lead to smaller and faster production code.

Raising an AssertionError in production code poses a security concern.

26

I’m bowing out, as I think I’m starting to get off topic when nitpicking Elis’ points. Fun talk and interesting suggestions, everyone. :slightly_smiling_face:

27

I usually just manually suppress these kinds of errors. I hate it when tooling that is supposed to help me gets in my way.

from enum import StrEnum


class Options(StrEnum):
    IDENTITY = "identity"
    DOUBLE = "double"
    SQUARE = "square"


def do_action(value: float, option: Options) -> float:
    match option:
        case Options.IDENTITY:
            new_value = value
        case Options.DOUBLE:
            new_value = value * 2
        case Options.SQUARE:
            new_value = value ** 2
    # noinspection PyUnboundLocalVariable
    return new_value
28

This is exactly why I personally don’t use type-checkers for Python and only write type annotations as a form of documentation. From seeing the experiences of others, I am strongly convinced that such tools would “get in my way” far more often than they would help me.

1 Like
29

Consider what happens when I (as a user or dev) call do_action(value=3, option='double'). Since 'double' (a mistaken input instead of Options.DOUBLE) does not appear in any of the match cases this code will result in a NameError on the return line saying NameError: name 'new_value' is not defined. What my approach and the other approaches are doing is replacing this NameError with an exception and message that is more useful to the user to help them see what they did wrong.

Also note that I’m asking in the context of pre-3.10 code, so match statements aren’t an option. But the same “pattern-level” question about raising a helpful warning or not persists.

Here the IDE is specifically telling the coder that there is a possible route through the code that allows such unhelpful NameError’s to arise. I’ve found this specific warning to be helpful in improving my code. But in fact, I find a VERY high percentage of the warning to help improve my code if I work to satisfy them. But I can see that that is a personal preference thing to some degree.

1 Like
30

A type hinter will catch this - Expected type 'Options', got 'str' instead

If you worry that the end-user of your function might ignore type hints warnings, then sure, the best option is to raise a ValueError in else or case _ clause.

31

Right, including the explicit ValueError (or whatever “options exhausted” error is appropriate) is sort of the minimal trust approach. It allows for the caller to make arbitrary mistakes and still strives to give back a useful message.

I agree that as you put more trust in the caller that you can remove this exception and start hiding pycharm inspection warnings.

1 Like
32

I want to share some more info that I’ve found on this because I think it will contribute to the exhaustiveness of the information in this thread.

First the python typing documentation has a specific section about this question of “exhaustiveness checking”: Unreachable Code and Exhaustiveness Checking — typing documentation.

Next, consider this real-life function from the library I linked above.

def get_pdg_round_digit(num: Decimal):
    top_digit = get_top_digit(num)

    # Bring num to be between 100 and 1000.
    num_top_three_digs = num * 10 ** (2 - top_digit)
    num_top_three_digs = round(num_top_three_digs, 0)
    new_top_digit = get_top_digit(num_top_three_digs)
    num_top_three_digs = num_top_three_digs * 10 ** (2 - new_top_digit)
    if 100 <= num_top_three_digs <= 354:
        round_digit = top_digit - 1
    elif 355 <= num_top_three_digs <= 949:
        round_digit = top_digit
    elif 950 <= num_top_three_digs <= 999:
        round_digit = top_digit
    else: 
        raise ValueError

    return round_digit

In this case, assuming the math is done right and get_top_digit works as expected, then num_top_three_digits will always be a number between 100 and 999. there is not even any user input that could be passed in to hit the ValueError. That code is truly unreachable. What should be done for this sort of code? I think the exhaustiveness checking doc above would recommend

...
else:
    assert False, "unreachable"

And this feels appropriate to me.

One final note. This came up in the context of trying to write test for these “unreachable” lines of code. For code that can be reached with mistaken user input the test is easy to write with assertRaises. But a line of code like I’ve shown above just can’t be tested I don’t think.

I’m using the coverage package to do test coverage. the docs indicate that lines like this can be excluded from test coverage calculations with a comment:

...
else:  # pragma: no cover
    assert False, "unreachable"
1 Like
33

Isn’t the summary basically:

So except in rare special situations where that doesn’t work for some specific reason, you should simply use that, no?

1 Like
34

I’m supporting versions of python that don’t have that in ‘stdlib’ and didn’t want to add an import. But I think you’re right. I didn’t fully understand the distinction between the ‘assert False’ and ‘assert_never’ yet.

1 Like
35

assert_never simply raises an AssertionError:

You can create a hard copy of this function.

1 Like
36

Using assert for runtime checks is a bug and should be avoided.

37

@elis.byberi, Why do you call this a runtime check? I’m suggesting to use this in code that can never be reached (no matter what user input is passed to the function) unless there is a bug in the library code. It’s being included to indicate to any reader of the code (or type checker) that all options have been exhausted.

1 Like
38

When Python is run in optimized mode, assert statements are stripped. Despite the intended purpose of being able to halt execution, using assert cannot achieve this in such cases.

In this case, function execution would be halted by a NameError exception anyway. I believe it would be preferable to raise a more informative exception, such as AssertionError(f"Expected code to be unreachable, but got: {value}")

39

I know that assert statements are stripped in optimize mode, I believe I referred to this above… Your posts are confusing me. I feel like you’re flipping your stances on things you’ve said previously.

Your most recent post makes it seem like you would NEVER advocate for using assert statements, which is a stance that I don’t necessarily disagree with. But I’m confused because higher up in the thread you were suggesting to use asserts instead of raising exceptions.

Also in your last post you seemed to be advocating FOR raising exceptions when a outcome outside of the expected exhausted options list is reached. But higher up you were advocating AGAINST that since it would be impossible to reach the code.

I can’t really tell what principles you are advocating for and against with each of your posts…

40

Never employ assert for runtime checks, especially when you’re uncertain about the code’s correctness.

If you’re uncertain about the code’s correctness, you should raise an informative exception.

def lowest_unused(street: str) -> int:
    for i in itertools.count(1):
        if not is_used(street, i):
            return i
    assert False, "unreachable"

You can use assert False, "unreachable" in the provided code because it’s certain that this section is not reachable (itertools.count() is an infinite iterator).

To sum up, utilize assert when it’s certain that a certain portion of code is unreachable, and opt for informative exceptions when there’s uncertainty about whether a section is reachable or not.

For instance, if there is substantial evidence that the get_pdg_round_digit function is correct, it’s appropriate to use assert ; otherwise, resort to raising an exception.