OCA / pylint-odoo

Odoo plugin for Pylint
http://www.pylint.org
143 stars 168 forks source link

Opinion: except-pass conflicts with Pythonic EAFP #361

Closed StefanRijnhart closed 2 years ago

StefanRijnhart commented 2 years ago

Having a warning in pylint-odoo about using pass after an exception (except-pass, added in #107) conflicts with the 'easier to ask forgiveness than to ask permission' approach (see https://docs.quantifiedcode.com/python-anti-patterns/readability/asking_for_permission_instead_of_forgiveness_when_working_with_files.html).

This approach typically catches specific exceptions which are then passed. The pylint-odoo recommendation to add a log message can lead to substantial, and overall redundant logging.

Would there be interest to remove this check? If so, I could propose a code change.

sbidoul commented 2 years ago

:+1: to remove it, at least from https://github.com/OCA/oca-addons-repo-template/blob/master/src/.pylintrc-mandatory.jinja

pedrobaeza commented 2 years ago

Yeah, I agree to remove it, but this will require an update of all repositories. I would take the occasion to update runboat and GH actions links with such update.

yajo commented 2 years ago

The reasoning is that, if there's an exception, something should have gone wrong. Thus, there has to be something to be done. Usually, at least, printing a log line explaining the something that is being skipped.

The modern pythonic way to avoid the lint error:

import contextlib
with contextlib.suppress(IndexError):
    # the code that would produce the error and still be OK
    recordset[0].unlink()

We can leave it in optional checks, as it is somewhat logic what the linter tries to say, although maybe too strict for a requirement.

StefanRijnhart commented 2 years ago

@Yajo thanks, I can live with replacing my try/except/pass clauses with this.