diff mbox series

[libgpiod,09/22] bindings: python: fix LineRequest union-attr type errors

Message ID 20240927-vfazio-mypy-v1-9-91a7c2e20884@xes-inc.com
State New
Headers show
Series bindings: python: conform to mypy and ruff linter recommendations | expand

Commit Message

Vincent Fazio Sept. 27, 2024, 6:53 p.m. UTC 
Since `LineRequest._req` can be `None`, it's necessary to inform type
checkers of the state of the object to silence the union-attr errors.

Type checkers may not be able to infer that an object is not `None` from
an earlier call (such as `_check_released`).

Instead of littering the code with "# type: ignore" comments, use casts
to inform type checkers that objects are not `None`.

Using `assert` is another option, however this duplicates the logic in
`_check_released` so is redundant at best and, at worst, is not a safe
replacement as `assert` can be elided in optimized Python environments
and these checks need to be runtime enforced.

Also, convert singular ints or strs to a tuple instead of a list to keep
with the inferred variable type of `lines`.

Signed-off-by: Vincent Fazio <vfazio@xes-inc.com>
---
 bindings/python/gpiod/line_request.py | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

Comments

Bartosz Golaszewski Oct. 8, 2024, 1:18 p.m. UTC | #1 
On Fri, Sep 27, 2024 at 8:57 PM Vincent Fazio <vfazio@xes-inc.com> wrote:
>
> Since `LineRequest._req` can be `None`, it's necessary to inform type
> checkers of the state of the object to silence the union-attr errors.
>
> Type checkers may not be able to infer that an object is not `None` from
> an earlier call (such as `_check_released`).
>
> Instead of littering the code with "# type: ignore" comments, use casts
> to inform type checkers that objects are not `None`.
>
> Using `assert` is another option, however this duplicates the logic in
> `_check_released` so is redundant at best and, at worst, is not a safe
> replacement as `assert` can be elided in optimized Python environments
> and these checks need to be runtime enforced.
>
> Also, convert singular ints or strs to a tuple instead of a list to keep
> with the inferred variable type of `lines`.
>
> Signed-off-by: Vincent Fazio <vfazio@xes-inc.com>
> ---

Basically the same questions as with Chip. Is this a common pattern in the wild?

Bart
diff mbox series

Patch

diff --git a/bindings/python/gpiod/line_request.py b/bindings/python/gpiod/line_request.py
index 77d199ac64e9d3cc68d4a8b38dd0f571a24ab231..6c57f612a4167061945e798e93f069689723b583 100644
--- a/bindings/python/gpiod/line_request.py
+++ b/bindings/python/gpiod/line_request.py
@@ -6,7 +6,7 @@  from __future__ import annotations
 from collections.abc import Iterable
 from datetime import timedelta
 from types import TracebackType
-from typing import Optional, Union
+from typing import Optional, Union, cast
 
 from . import _ext
 from ._internal import poll_fd
@@ -74,6 +74,7 @@  class LineRequest:
         not be used after a call to this method.
         """
         self._check_released()
+        self._req = cast(_ext.Request, self._req)
         self._req.release()
         self._req = None
 
@@ -114,6 +115,7 @@  class LineRequest:
           List of logical line values.
         """
         self._check_released()
+        self._req = cast(_ext.Request, self._req)
 
         lines = lines or self._lines
 
@@ -148,6 +150,7 @@  class LineRequest:
             Dictionary mapping line offsets or names to desired values.
         """
         self._check_released()
+        self._req = cast(_ext.Request, self._req)
 
         mapped = {
             self._name_map[line] if self._check_line_name(line) else line: values[line]
@@ -173,13 +176,14 @@  class LineRequest:
             Any settings for non-requested lines are ignored.
         """
         self._check_released()
+        self._req = cast(_ext.Request, self._req)
 
         line_cfg = _ext.LineConfig()
         line_settings = {}
 
         for lines, settings in config.items():
             if isinstance(lines, int) or isinstance(lines, str):
-                lines = [lines]
+                lines = (lines,)
 
             for line in lines:
                 offset = self._name_map[line] if self._check_line_name(line) else line
@@ -222,6 +226,7 @@  class LineRequest:
           List of read EdgeEvent objects.
         """
         self._check_released()
+        self._req = cast(_ext.Request, self._req)
 
         return self._req.read_edge_events(max_events)
 
@@ -275,4 +280,5 @@  class LineRequest:
         File descriptor associated with this request.
         """
         self._check_released()
+        self._req = cast(_ext.Request, self._req)
         return self._req.fd