Re: [libgpiod][PATCH 07/22] bindings: python: fix Chip union-attr type errors

[Bartosz Golaszewski <brgl@xxxxxxxx>]

On Tue, Oct 8, 2024 at 4:57 PM Vincent Fazio <vfazio@xxxxxxxxx> wrote:
>
> On Tue, Oct 8, 2024 at 8:16 AM Bartosz Golaszewski <brgl@xxxxxxxx> wrote:
> >
> >
> > Ok, so I don't really understand what is happening here. We're going
> > to re-assign _chip in every function? What happens to cast() if _chip
> > IS None?
>
> In this scenario, self._chip cannot be None. The self._check_closed() guard
> ensures this, however, type checkers (at least mypy) cannot infer that from the
> current code pattern.
>
> `cast` is informing the type checker that past this point, self._chip should
> not be considered as None and it's safe to reference attributes off the object
>
> This seemed like the cleanest alternative, though others are:
>
> * use a local variable for the cast result. This may be less confusing but
>   incurs more changed lines
>
>     self._check_closed()
>     chip = cast(_ext.Chip, self._chip)
>     return chip.path
>

For the sake of readability, I would lean more towards this option if
I'm honest.

Or even - if you need to use the cast variable only once:

self._check_closed()
return cast(_ext.Chip, self._chip).path

?

> * use asserts. These aren't always enforced during runtime so we cannot replace
>   _check_closed but it does inform the type checker that it can narrow the type.
>   Using both is ok, just slightly redundant.
>
>     self._check_closed()
>     assert self._chip is not None
>     return self._chip.path
>

Yeah, this isn't optimal.

> * add a `if self._chip is None` check that duplicates _check_closed
> (or replace it completely)
>

Yep, no.

> * other "creative" solutions like a wrapper that returns a Chip or
> throws an exception if it's None.
>
>     def _chip_or_exc(self) -> Chip:
>         if self._chip is None:
>             raise Exception
>         return self._chip
>
>     chip = self._chip_or_exc()
>     return chip.path
>

Ugh. I see, cast() is the best solution.

Please consider the small change I suggested.

Bart