Improving the Python code quality of the team with pylint (and ChatGPT)
Published: 2023-06-20 13:16:13

> ℹ️ Note
> 
> This is the english version of [the original post, in Catalan](https://dev.to/chaosct/augmentant-la-qualitat-de-codi-python-de-lequip-amb-pylint-i-chatgpt-26d8). 
> 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`.

```python

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:

```python

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`:

```python

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, `assert`s 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:

```python

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.

```python
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.