[Adjust Rule] SIM106 False-Positive for NotImplementedError / ValueError

[martin-thoma]

Desired change

• Rule(s): SIM106
• Adjustment: Remove false-positive for NotImplementedError

Explanation

It's pretty typical to exhaustively deal with some property. Just in case there might be a missed one, throw an Exception.

Example

if image_extension in ['.jpg', '.jpeg']:
    return 'JPEG'
elif image_extension in ['.png']:
    return 'PNG'
else:
    raise ValueError("Unknwon image extension {image extension}")

[jonyscathe]

This is a great addition, definitely allowed the removal of a few noqa: SIM106 comments we had in our code. What are people's thoughts on extended this to TypeError as well? We have some instances where there is different behaviour depending on one of 5 or 6 different types that an object could be. Having an error check at the top for that situation is a bit messy and it is easy to forget to update when any new type is added to the code or a type is removed.

[MartinThoma]

I'm becoming more and more certain that I want to remove SIM106 entirely. What do you think @jonyscathe : Did SIM106 provide value so far or was it just an annoyance?

[jonyscathe]

It is a tricky one. I agree that most of the time that error handling should be done at the beginning of the block. Especially when checking validity of function inputs or something along those lines. So SIM106 was useful for this However, when there is something that is effectively a switch statement made up with an if and a lot of elif then firstly handling the logical 'else' (or in switch statement language, 'default') case first is going to be messy. Plus even if I do that I will still want an else block just to check that I haven't missed any case.

Then again, we don't have a lot of these, and at some point in the future we'll probably shift those over to the new match/case statements for readability anyway.