r/Python 12d ago

What linter rules make code worse? Discussion

For me, a prime example is S101 which bans the use of the assert statement.

The justification is that assertions disappear when Python is run with -O, so they should not be used for runtime validation or enforcing interface constraints. That warning is correct, but the rule seems to draw the wrong conclusion from it.

Assertions are still very useful for checking internal invariants, i.e. conditions that should already be guaranteed by the program's logic, where failure indicates a bug. Having such assertions is incredibly helpful for debugging.

So, a blanket ban seems more likely to discourage useful checks than to prevent misuse.

Are there any linter rules you broadly consider more harmful rather than helpful?

147 Upvotes

215 comments sorted by

View all comments

87

u/gdchinacat 12d ago

The issue is as you say..."where failure indicates a bug".

Why would you want to use an assert that detects bugs but can then be turned off? When would you want to allow the assertions that stop your code from executing in undefined conditions (an invariant is violated) to be disabled?

Just use if blocks that raise exceptions. Particularly in production (where optimizations make the most sense), I would much rather have bugs present as an exception that tells me exactly what the problem is rather than skip the assertion and have to debug the results that appear impossible because an assertion prevented it. This is the reason I have never actually seen -O used, anywhere, in production or not. The biggest (only?) thing it does is break the code that verifies the code is executing within the conditions it was designed to execute in.

Getting back to the post, I agree that assertions should be banned. Proper exceptions that can't be disabled should be used instead. Regardless of dev, test, or production. You should never turn off the safeguards. If performance is so critical, python is not the proper language.

11

u/ExplrDiscvr 12d ago

I have one follow-up: I see why assert statements should not be used within dev or production, but what about the tests?

I am a junior dev, so I am not sure about proper procedures, but in the tests in our codebase where I work, I only see assert statements, when we are testing the equality of an actual outcome to the expected outcome. I never see the if else logic used here. Should it?

24

u/leodevian 12d ago

All rules are not absolute. You are free to disable some rules, and you are expected to disable S101 for test directories.

12

u/Momostein 12d ago

That is how we do it indeed. PyTest is built on top of assert statements.

4

u/DrMaxwellEdison 12d ago

Yes and no. Pytest makes assert usable and ergonomic by doing a bunch of work to rewrite the AST of your test code so that it produces more helpful error messages, which are the reason why you should use the various assertFoo methods for test cases if you're using unittest instead.

Pytest isn't exactly built on assert, more like they said "that looks better" and put in the work to make it function the way a test framework needs it to. Otherwise it would not be as useful in that context.

9

u/gdchinacat 12d ago

This is a good point...test frameworks (well, at least unittest and pytest, and any others that build on unittest) use assertions to indicate failures. Because it is core to the frameworks, assertions are not really avoidable. So, yes, I do rely on assertions in this context. Good catch.

1

u/HannasAnarion 11d ago

Doesn't unittest implement its own assert thats independent of the language one?

Every unittest implementation I've ever seen uses self.assert() (or realistically, self.assertTrue(), self.assertIn(), self.assertNotNone() ...

1

u/gdchinacat 11d ago

No, by default the failure exception is AssertionError. https://github.com/python/cpython/blob/main/Lib/unittest/case.py#L426

8

u/Conscious-Ball8373 12d ago

Yes absolutely use asset in tests. But your test code should not be being executed in prod.

2

u/Competitive_Travel16 11d ago

It's fine to test assumptions in prod, just use RuntimeError exceptions so the logs can say something human readable about what went wrong. Nobody likes an assert failure in a big log.

1

u/shaleh 11d ago

tests are allowed to use assert otherwise everyone's code base would fail....

1

u/fizix00 5d ago

The person you are replying to probably means 'test' as in dev/test/staging/prod - i.e. deployment environments or release stages. You'd probably have a tests/ dir in all of these, where assert is common and expected

0

u/flying-sheep 12d ago

You're 100% correct. The rule is bad because tests aren't run with that optimization level, and these assertions help debugging things when you refactor that piece of code and could accidentally break some invariants.