DEV Community: Carles Julià

Improving the Python code quality of the team with pylint (and ChatGPT)

Carles Julià — Tue, 20 Jun 2023 13:16:13 +0000

ℹ️ Note

This is the english version of the original post, in Catalan.
It was translated using ChatGPT (GPT-4).

Modern Development in Python

Python is not what it used to be. For a while now, modern development with Python involves a whole set of tools that help us improve code quality.

Linting projects such as pylint, and more recently ruff, have been analyzing code statically to find issues for some time.

More recently, mypy helps us verify that the code is correct through an explicit declaration of types.

And to top it off, pre-commit provides a way to automate these tools to run regularly.

All of these tools turn out to be essential in the day-to-day life of the development team.

The Problem

If you have experience with mypy, you have probably encountered situations where it does not behave as you expected. The temptation to add a # ignore: typing is great, or to use typing.cast to remove the error. After all, if we don't satisfy mypy, we won't pass the pre-commit.


import typing

class A:
    def a():
        print('a')

class B(A):
    def b():
        print('b')

things: dict[str,A] = {'a':A(), 'b':B()}

b = things['b']
# We know that b is an instance of B, but mypy doesn't

b.b()  # mypy doesn't like this

typing.cast(B, b).b()  # mypy accepts the change

Although this satisfies mypy, it is very dangerous. What if the part of the code changes and b ends up being from class A? It could possibly lead to runtime errors, but in less evident places. If instead of directly calling b.b() we passed it to another function, it could go unnoticed for a long time:


from queue import Queue

cua: Queue[B] = Queue()

def do_something_later(b: B) -> None:
    cua.put(b)

b_b = typing.cast(B, b)  # mypy accepts the change
do_something_later(b_b)

Then we will not see the error until the element is consumed from the queue.

Alternatives

The simplest alternative to typing.cast is assert isinstance:


b = things['b']
# We know that b is an instance of B, but mypy doesn't
assert isinstance(b,B)  # now mypy does
b.b()  # No problem

If you notice, the code reads: "I, the programmer, assure you that I know b is an instance of B, and if I'm wrong, let an exception be raised!". This prevents us from experiencing what happened in the previous example where the error went undetected for a long time.

Also, in a production environment, asserts can be disabled, eliminating any cost overhead.

However, this does not work for types like dict[str,B]. To solve this, we can use pydantic and create a replacement command:


from typing import cast
from pydantic import parse_obj_as

def assert_cast(expected_type: type[T], value: Any) -> T:
    """Drop-in replacement for typing.cast, but with runtime checks.
    If assert is disabled, this function does nothing.
    Checks that value is of type expected_type and returns it as such.
    this also works with arbitrary types such as dict[...], Pydantic models, etc.
    Because of how Pydantic works, we can't prevent the check to use type coercion.
    This should be solved in Pydantic V2, in the future"""

    # If asserts are disabled, we don't want to do anything

    if not __debug__:
        return cast(T, value)

    try:
        parse_obj_as(expected_type, value)
    except ValueError as exc:
        raise AssertionError(
            f"Expected value of type {expected_type} but got {type(value)} instead"
        ) from exc
    return cast(T, value)

Now replace cast with assert_cast, and you will have all the benefits of cast and assert at the same time.

assert_cast(dict[str,B],b_dict)

Note: Remember that you can also ask ChatGPT to generate unit tests for the function. And remember to review them!

Implementation

Once we have identified the problem in our codebase, we need to find a way to prevent it from happening again. Obviously, we can explain it to the team members, but we cannot rely solely on memory; it will happen again.

We would need to find a way to detect typing.cast in the pre-commit to prevent it from slipping through, but how? Well, let's make a pylint plugin.

The problem is that writing a pylint plugin seems to be a long and boring task. And, worst of all, we would become experts in pylint plugins and would be constantly asked to write them. Nobody wants that.

Therefore, the best alternative is to entrust it to ChatGPT.

ChatGPT

For ChatGPT to make us a pylint plugin we have to ask for it:

Please write a pylint plugin that warns about using typing.cast function anywhere in the code.

For best results, it's better to use GPT-4, GPT-3.5 hallucinates too much.

On the first attempt, it will generate correct code, but it will miss some cases. For example, initially it only detected when typing.cast was called but not when cast was called after doing from typing import cast. The best approach top solve that is to keep asking ChatGPT to add these cases.

It helps a lot to have various canary files where we deliberately add typing.cast in different ways, to verify that the plugin indeed recognizes them.

Once it works, it's very helpful to make a document in the same repository explaining the whole rationale for banning the use of typing.cast and referring to it in the warning message. That way, everyone will be able to understand what's going on when the error pops up.

Recovering the `pre-commit`

Once the plugin is added, the pre-commit will fail, finding all the times typing.cast appears in the code.

We find ourselves in the situation where we would have to solve all pylint errors before being able to push the rule into th repository, and this can be a too big of a task.

In this case, the strategy we chose was to add the pylint rule as a warning and not an error, so that over time, we can make the change and progressively replace the cases of typing.cast.

Conclusions

pylint and pre-commit are fantastic for this use case: enforcing a rule that bans the use of a function. This way we don't have to manually monitor that people don't skip it.

We discovered that ChatGPT is very effective for this task.

If you can't get the pre-commit green in one go, you will need to organize sessions to finish eliminating the banned function.

Augmentant la qualitat de codi Python de l'equip amb pylint (i ChatGPT)

Carles Julià — Mon, 12 Jun 2023 08:09:35 +0000

Desenvolupament modern en Python

Python ja no és el que era. Ja fa un temps que el desenvolupament modern amb Python involucra tot d'eines que ens ajuden a augmentar la qualitat del codi.

Ja de temps enrere, projectes de linting com pylint, i més recentment ruff, analitzen el codi de forma estàtica per a trobar-ne problemes.

Més recentment mypy ens ajuda a comprovar que el codi sigui correcte a traves de una declaració explícita dels tipus.

I per a reblar el clau, pre-commit ens dóna la forma d'automatitzar aquestes eines per a que s'executin de forma regular.

Totes aquestes eines resulten ser essencials en el dia a dia de l'equip de desenvolupament.

El problema

Si teniu experiencia amb mypy, segur que us haureu trobat amb situacions on no es comporta com esperàveu. La temptació de afegi un # ignore: typing és gran, o també de fer servir typing.cast per a treure l'error. Al cap i a la fi, si no aconseguim acontentar a mypy no ens passarà el pre-commit .

import typing

class A:
    def a():
        print('a')

class B(A):
    def b():
        print('b')

things: dict[str,A] = {'a':A(), 'b':B()}

b = things['b']
# Sabem que b és instancia de B, però mypy no

b.b()  # Això no li agrada a mypy

typing.cast(B, b).b()  # mypy accepta el canvi

Tot i que això acontenta a mypy, és molt perillós. I si la part del codi canvia i b acaba sent de la classe A? És possible que ens provoqui errors en temps d'execució, però en llocs poc evidents. Si en lloc de cridar directament b.b() ho passéssim a una altra funció, podria passar desapercebut durant molt de temps:

from queue import Queue

cua: Queue[B] = Queue()

def fes_algo_mes_tard(b: B) -> None:
    cua.put(b)

b_b = typing.cast(B, b)  # mypy accepta el canvi
fes_algo_mes_tard(b_b)

Llavors no veurem l'error fins que l'element sigui consumit de la cua.

Alternatives

L'alternativa més senzilla a typing.cast és assert isinstance:

b = things['b']
# Sabem que b és instancia de B, però mypy no
assert isinstance(b,B)  # ara mypy sí que ho sap
b.b()  # Cap problema

Si us hi fixeu, el codi llegeix: "Jo, programador, us asseguro que sé que b és instància de B, i si no tinc raó, que salti una excepció!". Això ens impedeix que ens passi com en l'exemple anterior on l'error quedava sense detectar durant temps.

A més, en entorn de producció es poden desactivar els asserts, eliminant qualsevol penalització en l'eficiència.

Així i tot, això no funciona per tipus com dict[str,B]. Per a solucionar-ho podem usar pydantic i crear una comanda de substitució:

from typing import cast
from pydantic import parse_obj_as

def assert_cast(expected_type: type[T], value: Any) -> T:
    """Drop-in replacement for typing.cast, but with runtime checks.
    If assert is disabled, this function does nothing.
    Checks that value is of type expected_type and returns it as such.
    this also works with arbitrary types such as Union, Pydanitc models, etc.
    Because of how Pydantic works, we can't prevent the check to use type coercion.
    This should be solved in Pydantic V2, in the future"""

    # If asserts are disabled, we don't want to do anything
    if not __debug__:
        return cast(T, value)

    try:
        parse_obj_as(expected_type, value)
    except ValueError as exc:
        raise AssertionError(
            f"Expected value of type {expected_type} but got {type(value)} instead"
        ) from exc

    return cast(T, value)

I ara substituïu cast per assert_cast, i així tindreu tots els beneficis de cast i assert alhora.

assert_cast(dict[str,B],b_dict)

Nota: Recordeu que també podeu demanar a ChatGPT que generi tests unitaris per la funció. I recordeu-vos de revisar-los!

Implantació

Un cop tenim identificat el problema en la nostra base de codi, cal trobar una forma perquè no torni a passar. Evidentment, se'ls pot explicar als membres de l'equip, però no podem confiar només en la memòria; tornarà a passar.

Caldria trobar la forma de detectar typing.cast en el pre-commit per a evitar que se'ns passi, però com? Doncs fem un plugin de pylint.

El problema és que fer un plugin de pylint té pinta de ser una tasca llarga i avorrida. I, pitjor de tot, ens convertiríem en experts en plugins de pylint i ens n'encarregarien constantment. Ningú ho vol, això.

Per tant, la millor alternativa és encarregar-ho a ChatGPT.

ChatGPT

Perquè ChatGPT ens faci un plugin de pylint li ho hem d'encarregar:

Please write a pylint plugin that warns about using typing.cast function anywhere in the code.

Per a tenir uns bons resultats, millor usem GPT-4, GPT-3.5 al·lucina massa.

Al primer intent, ens generarà codi correcte, però que es deixa alguns casos. Per exemple, al principi només ens detectava quan es cridava typing.cast però no quan es cridava castdesprés de fer from typing import cast. La tècnica és anar encarregant-li al ChatGPT que afegeixi aquests casos.

Ajuda molt tenir diversos fitxers canaris on afegim deliberadament typing.cast de diferents formes, per a comprovar que efectivament, el plugin els reconeix.

Un cop funcionant, ajuda molt fer un document en el mateix repositori on explica tot el raonament de per què es prohibeix l'ús de typing.cast i referir-s'hi en el missatge del warning. D'aquesta forma tothom podrà entendre de què va el tema quan li salti l'error.

Recuperant el `pre-commit`

Un cop afegit el plugin, el pre-commit ens fallarà, trobant totes les vegades que hi ha typing.cast en el codi.

Ens trobem en la situació que hauríem de solucionar tots els errors de pylint abans de poder fer push de la regla, i això pot ser una tasca massa gran, si n'hi ha masses.

En aquest cas, l'estratègia que vam triar va ser la de afegir la regla de pylint com a warning i no com a error, de forma que, al llarg del temps, podem fer el canvi i anar substituint els casos de typing.castde forma progressiva.

Conclusions

pylint i pre-commit són fantàstics per aquest cas d'ús: reforçar una norma que prohibeix l'ús d'una funció. D'aquesta manera no cal vigilar manualment que la gent no se la salti.

Un descobriment és com d'efectiu és ChatGPT per a aquesta tasca.

Si no es pot deixar el pre-commit en verd d'una sola passada, caldrà organitzar sessions per acabar d'eliminar la funció prohibida.

DEV Community: Carles Julià

Improving the Python code quality of the team with pylint (and ChatGPT)

Modern Development in Python

The Problem

Alternatives

Implementation

ChatGPT

Recovering the pre-commit

Conclusions

Augmentant la qualitat de codi Python de l'equip amb pylint (i ChatGPT)

Desenvolupament modern en Python

El problema

Alternatives

Implantació

ChatGPT

Recuperant el pre-commit

Conclusions

Recovering the `pre-commit`

Recuperant el `pre-commit`