[libgpiod] [RFC PATCH] bindings: python: allow specifying infinite timeout

[libgpiod] [RFC PATCH] bindings: python: allow specifying infinite timeout

[Nicolas Frattaroli]

So far, libgpiod's Python bindings had no way to state that a
user wishes to wait for events indefinitely, as a timeout of
None would intentionally be converted to 0 seconds, i.e. return
from the select call in poll_fd immediately.

The usual Python convention and even the select convention is
to block indefinitely on a timeout=None. However, changing the
poll_fd function to do this now would change an (intentional)
API design choice by libgpiod 2.0 that API users presumably
rely on.

By allowing float("inf") (or in fact math.inf, or your favourite
other way to get an infinite float) to mean waiting infinitely
solves this by extending the API rather than changing it.

On gpiod Python bindings without this change, passing inf results
in an OverflowError being raised in select. API users who wish to
support older versions of the bindings can catch this exception and
act on it.

Signed-off-by: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
---
 bindings/python/gpiod/chip.py         | 3 ++-
 bindings/python/gpiod/internal.py     | 4 ++++
 bindings/python/gpiod/line_request.py | 3 ++-
 3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/bindings/python/gpiod/chip.py b/bindings/python/gpiod/chip.py
index 97ff340..95c5757 100644
--- a/bindings/python/gpiod/chip.py
+++ b/bindings/python/gpiod/chip.py
@@ -195,7 +195,8 @@ class Chip:
         Args:
           timeout:
             Wait time limit represented as either a datetime.timedelta object
-            or the number of seconds stored in a float.
+            or the number of seconds stored in a float. A timeout of None
+            returns immediately, use float("inf") to wait indefinitely.
 
         Returns:
           True if an info event is ready to be read from the chip, False if the
diff --git a/bindings/python/gpiod/internal.py b/bindings/python/gpiod/internal.py
index 37e8b62..141cfe9 100644
--- a/bindings/python/gpiod/internal.py
+++ b/bindings/python/gpiod/internal.py
@@ -2,6 +2,7 @@
 # SPDX-FileCopyrightText: 2022 Bartosz Golaszewski <brgl@bgdev.pl>
 
 from datetime import timedelta
+from math import inf
 from select import select
 from typing import Optional, Union
 
@@ -15,5 +16,8 @@ def poll_fd(fd: int, timeout: Optional[Union[timedelta, float]] = None) -> bool:
     else:
         sec = timeout
 
+    if sec == inf:
+        sec = None
+
     readable, _, _ = select([fd], [], [], sec)
     return True if fd in readable else False
diff --git a/bindings/python/gpiod/line_request.py b/bindings/python/gpiod/line_request.py
index a0f97b7..ae21835 100644
--- a/bindings/python/gpiod/line_request.py
+++ b/bindings/python/gpiod/line_request.py
@@ -178,7 +178,8 @@ class LineRequest:
         Args:
           timeout:
             Wait time limit expressed as either a datetime.timedelta object
-            or the number of seconds stored in a float.
+            or the number of seconds stored in a float. None returns
+            immediately. Use float("inf") to wait indefinitely.
 
         Returns:
           True if events are ready to be read. False on timeout.
-- 
2.40.1

Re: [libgpiod] [RFC PATCH] bindings: python: allow specifying infinite timeout

[Bartosz Golaszewski]

On Fri, May 19, 2023 at 7:47 PM Nicolas Frattaroli
<frattaroli.nicolas@gmail.com> wrote:
>
> So far, libgpiod's Python bindings had no way to state that a
> user wishes to wait for events indefinitely, as a timeout of
> None would intentionally be converted to 0 seconds, i.e. return
> from the select call in poll_fd immediately.
>
> The usual Python convention and even the select convention is
> to block indefinitely on a timeout=None. However, changing the
> poll_fd function to do this now would change an (intentional)
> API design choice by libgpiod 2.0 that API users presumably
> rely on.
>
> By allowing float("inf") (or in fact math.inf, or your favourite
> other way to get an infinite float) to mean waiting infinitely
> solves this by extending the API rather than changing it.
>
> On gpiod Python bindings without this change, passing inf results
> in an OverflowError being raised in select. API users who wish to
> support older versions of the bindings can catch this exception and
> act on it.
>
> Signed-off-by: Nicolas Frattaroli <frattaroli.nicolas@gmail.com>
> ---
>  bindings/python/gpiod/chip.py         | 3 ++-
>  bindings/python/gpiod/internal.py     | 4 ++++
>  bindings/python/gpiod/line_request.py | 3 ++-
>  3 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/bindings/python/gpiod/chip.py b/bindings/python/gpiod/chip.py
> index 97ff340..95c5757 100644
> --- a/bindings/python/gpiod/chip.py
> +++ b/bindings/python/gpiod/chip.py
> @@ -195,7 +195,8 @@ class Chip:
>          Args:
>            timeout:
>              Wait time limit represented as either a datetime.timedelta object
> -            or the number of seconds stored in a float.
> +            or the number of seconds stored in a float. A timeout of None
> +            returns immediately, use float("inf") to wait indefinitely.
>
>          Returns:
>            True if an info event is ready to be read from the chip, False if the
> diff --git a/bindings/python/gpiod/internal.py b/bindings/python/gpiod/internal.py
> index 37e8b62..141cfe9 100644
> --- a/bindings/python/gpiod/internal.py
> +++ b/bindings/python/gpiod/internal.py
> @@ -2,6 +2,7 @@
>  # SPDX-FileCopyrightText: 2022 Bartosz Golaszewski <brgl@bgdev.pl>
>
>  from datetime import timedelta
> +from math import inf
>  from select import select
>  from typing import Optional, Union
>
> @@ -15,5 +16,8 @@ def poll_fd(fd: int, timeout: Optional[Union[timedelta, float]] = None) -> bool:
>      else:
>          sec = timeout
>
> +    if sec == inf:
> +        sec = None
> +
>      readable, _, _ = select([fd], [], [], sec)
>      return True if fd in readable else False
> diff --git a/bindings/python/gpiod/line_request.py b/bindings/python/gpiod/line_request.py
> index a0f97b7..ae21835 100644
> --- a/bindings/python/gpiod/line_request.py
> +++ b/bindings/python/gpiod/line_request.py
> @@ -178,7 +178,8 @@ class LineRequest:
>          Args:
>            timeout:
>              Wait time limit expressed as either a datetime.timedelta object
> -            or the number of seconds stored in a float.
> +            or the number of seconds stored in a float. None returns
> +            immediately. Use float("inf") to wait indefinitely.
>
>          Returns:
>            True if events are ready to be read. False on timeout.
> --
> 2.40.1
>

I like this approach too. In fact - it may be even clearer and more
intuitive than converting None to infinite timeout.

Any objections against using negative numbers for the same purpose as well?

Bart

Re: [libgpiod] [RFC PATCH] bindings: python: allow specifying infinite timeout

[andy.shevchenko]

Tue, May 23, 2023 at 12:06:47PM +0200, Bartosz Golaszewski kirjoitti:
> On Fri, May 19, 2023 at 7:47 PM Nicolas Frattaroli
> <frattaroli.nicolas@gmail.com> wrote:

> > So far, libgpiod's Python bindings had no way to state that a
> > user wishes to wait for events indefinitely, as a timeout of
> > None would intentionally be converted to 0 seconds, i.e. return
> > from the select call in poll_fd immediately.
> >
> > The usual Python convention and even the select convention is
> > to block indefinitely on a timeout=None. However, changing the
> > poll_fd function to do this now would change an (intentional)
> > API design choice by libgpiod 2.0 that API users presumably
> > rely on.
> >
> > By allowing float("inf") (or in fact math.inf, or your favourite
> > other way to get an infinite float) to mean waiting infinitely
> > solves this by extending the API rather than changing it.
> >
> > On gpiod Python bindings without this change, passing inf results
> > in an OverflowError being raised in select. API users who wish to
> > support older versions of the bindings can catch this exception and
> > act on it.

...

> I like this approach too. In fact - it may be even clearer and more
> intuitive than converting None to infinite timeout.

With all respect to the clever design solutions I would rather go the
de facto Pythonic way. If the native libraries use None for indefinite
then it's better to do that way, otherwise we will add quite a confusion
to the Python users.

> Any objections against using negative numbers for the same purpose as well?

The question here is: What in the very same situations are other (presumably
native) Python libraries using?

-- 
With Best Regards,
Andy Shevchenko

Re: [libgpiod] [RFC PATCH] bindings: python: allow specifying infinite timeout

[Bartosz Golaszewski]

On Tue, May 23, 2023 at 2:18 PM <andy.shevchenko@gmail.com> wrote:
>
> Tue, May 23, 2023 at 12:06:47PM +0200, Bartosz Golaszewski kirjoitti:
> > On Fri, May 19, 2023 at 7:47 PM Nicolas Frattaroli
> > <frattaroli.nicolas@gmail.com> wrote:
>
> > > So far, libgpiod's Python bindings had no way to state that a
> > > user wishes to wait for events indefinitely, as a timeout of
> > > None would intentionally be converted to 0 seconds, i.e. return
> > > from the select call in poll_fd immediately.
> > >
> > > The usual Python convention and even the select convention is
> > > to block indefinitely on a timeout=None. However, changing the
> > > poll_fd function to do this now would change an (intentional)
> > > API design choice by libgpiod 2.0 that API users presumably
> > > rely on.
> > >
> > > By allowing float("inf") (or in fact math.inf, or your favourite
> > > other way to get an infinite float) to mean waiting infinitely
> > > solves this by extending the API rather than changing it.
> > >
> > > On gpiod Python bindings without this change, passing inf results
> > > in an OverflowError being raised in select. API users who wish to
> > > support older versions of the bindings can catch this exception and
> > > act on it.
>
> ...
>
> > I like this approach too. In fact - it may be even clearer and more
> > intuitive than converting None to infinite timeout.
>
> With all respect to the clever design solutions I would rather go the
> de facto Pythonic way. If the native libraries use None for indefinite
> then it's better to do that way, otherwise we will add quite a confusion
> to the Python users.
>
> > Any objections against using negative numbers for the same purpose as well?
>
> The question here is: What in the very same situations are other (presumably
> native) Python libraries using?
>

As has been said elsewhere - the pythonic way is to interpret None as
indefinite timeout. It's just that it would change the current
behavior. The question is - should we interpret the current behavior
as "undefined" and change it, or "defined but not documented" and
consider it part of the API.

Bart

Re: [libgpiod] [RFC PATCH] bindings: python: allow specifying infinite timeout

[Nicolas Frattaroli]

On Dienstag, 23. Mai 2023 14:33:12 CEST Bartosz Golaszewski wrote:
> On Tue, May 23, 2023 at 2:18 PM <andy.shevchenko@gmail.com> wrote:
> >
> > Tue, May 23, 2023 at 12:06:47PM +0200, Bartosz Golaszewski kirjoitti:
> > > On Fri, May 19, 2023 at 7:47 PM Nicolas Frattaroli
> > > <frattaroli.nicolas@gmail.com> wrote:
> >
> > > > So far, libgpiod's Python bindings had no way to state that a
> > > > user wishes to wait for events indefinitely, as a timeout of
> > > > None would intentionally be converted to 0 seconds, i.e. return
> > > > from the select call in poll_fd immediately.
> > > >
> > > > The usual Python convention and even the select convention is
> > > > to block indefinitely on a timeout=None. However, changing the
> > > > poll_fd function to do this now would change an (intentional)
> > > > API design choice by libgpiod 2.0 that API users presumably
> > > > rely on.
> > > >
> > > > By allowing float("inf") (or in fact math.inf, or your favourite
> > > > other way to get an infinite float) to mean waiting infinitely
> > > > solves this by extending the API rather than changing it.
> > > >
> > > > On gpiod Python bindings without this change, passing inf results
> > > > in an OverflowError being raised in select. API users who wish to
> > > > support older versions of the bindings can catch this exception and
> > > > act on it.
> >
> > ...
> >
> > > I like this approach too. In fact - it may be even clearer and more
> > > intuitive than converting None to infinite timeout.
> >
> > With all respect to the clever design solutions I would rather go the
> > de facto Pythonic way. If the native libraries use None for indefinite
> > then it's better to do that way, otherwise we will add quite a confusion
> > to the Python users.
> >
> > > Any objections against using negative numbers for the same purpose as well?
> >
> > The question here is: What in the very same situations are other (presumably
> > native) Python libraries using?
> >
> 
> As has been said elsewhere - the pythonic way is to interpret None as
> indefinite timeout. It's just that it would change the current
> behavior. The question is - should we interpret the current behavior
> as "undefined" and change it, or "defined but not documented" and
> consider it part of the API.
> 
> Bart
> 

As an alternate suggestion, we could change the default function argument
to 0.0 and remove the None -> 0 code. That way, people who were calling
the function with no arguments still get the same behaviour, and the
only break is for users who explicitly passed None.

Kind regards,
Nicolas Frattaroli

Re: [libgpiod] [RFC PATCH] bindings: python: allow specifying infinite timeout

[Bartosz Golaszewski]

On Tue, May 23, 2023 at 2:38 PM Nicolas Frattaroli
<frattaroli.nicolas@gmail.com> wrote:
>
> On Dienstag, 23. Mai 2023 14:33:12 CEST Bartosz Golaszewski wrote:
> > On Tue, May 23, 2023 at 2:18 PM <andy.shevchenko@gmail.com> wrote:
> > >
> > > Tue, May 23, 2023 at 12:06:47PM +0200, Bartosz Golaszewski kirjoitti:
> > > > On Fri, May 19, 2023 at 7:47 PM Nicolas Frattaroli
> > > > <frattaroli.nicolas@gmail.com> wrote:
> > >
> > > > > So far, libgpiod's Python bindings had no way to state that a
> > > > > user wishes to wait for events indefinitely, as a timeout of
> > > > > None would intentionally be converted to 0 seconds, i.e. return
> > > > > from the select call in poll_fd immediately.
> > > > >
> > > > > The usual Python convention and even the select convention is
> > > > > to block indefinitely on a timeout=None. However, changing the
> > > > > poll_fd function to do this now would change an (intentional)
> > > > > API design choice by libgpiod 2.0 that API users presumably
> > > > > rely on.
> > > > >
> > > > > By allowing float("inf") (or in fact math.inf, or your favourite
> > > > > other way to get an infinite float) to mean waiting infinitely
> > > > > solves this by extending the API rather than changing it.
> > > > >
> > > > > On gpiod Python bindings without this change, passing inf results
> > > > > in an OverflowError being raised in select. API users who wish to
> > > > > support older versions of the bindings can catch this exception and
> > > > > act on it.
> > >
> > > ...
> > >
> > > > I like this approach too. In fact - it may be even clearer and more
> > > > intuitive than converting None to infinite timeout.
> > >
> > > With all respect to the clever design solutions I would rather go the
> > > de facto Pythonic way. If the native libraries use None for indefinite
> > > then it's better to do that way, otherwise we will add quite a confusion
> > > to the Python users.
> > >
> > > > Any objections against using negative numbers for the same purpose as well?
> > >
> > > The question here is: What in the very same situations are other (presumably
> > > native) Python libraries using?
> > >
> >
> > As has been said elsewhere - the pythonic way is to interpret None as
> > indefinite timeout. It's just that it would change the current
> > behavior. The question is - should we interpret the current behavior
> > as "undefined" and change it, or "defined but not documented" and
> > consider it part of the API.
> >
> > Bart
> >
>
> As an alternate suggestion, we could change the default function argument
> to 0.0 and remove the None -> 0 code. That way, people who were calling
> the function with no arguments still get the same behaviour, and the
> only break is for users who explicitly passed None.
>

Honestly, if we were to change the behavior, then I'd prefer to do it
right and not use any half-measures.

Bart

Re: [libgpiod] [RFC PATCH] bindings: python: allow specifying infinite timeout

[Nicolas Frattaroli]

On Dienstag, 23. Mai 2023 15:04:54 CEST Bartosz Golaszewski wrote:
> On Tue, May 23, 2023 at 2:38 PM Nicolas Frattaroli
> <frattaroli.nicolas@gmail.com> wrote:
> >
> > On Dienstag, 23. Mai 2023 14:33:12 CEST Bartosz Golaszewski wrote:
> > >
> > > ...
> > >
> > > As has been said elsewhere - the pythonic way is to interpret None as
> > > indefinite timeout. It's just that it would change the current
> > > behavior. The question is - should we interpret the current behavior
> > > as "undefined" and change it, or "defined but not documented" and
> > > consider it part of the API.
> > >
> > > Bart
> > >
> >
> > As an alternate suggestion, we could change the default function argument
> > to 0.0 and remove the None -> 0 code. That way, people who were calling
> > the function with no arguments still get the same behaviour, and the
> > only break is for users who explicitly passed None.
> >
> 
> Honestly, if we were to change the behavior, then I'd prefer to do it
> right and not use any half-measures.
> 
> Bart
> 

That's fair, I don't mind either way. I don't think the behavioural
change would be that big of an issue either way; the obviousness of a
wait on events function that returns immediately by default is
debatable in my eyes, as the only use for using it like that I could
ever come up with is to peek at whether there are any events queued
up based on its return value.

What I do think is a bad solution now is the float("inf") stuff.
While it's fun to assign meaning to special floating point values,
I think it strictly makes the API worse for the sake of stability.

Kind regards,
Nicolas Frattaroli

Re: [libgpiod] [RFC PATCH] bindings: python: allow specifying infinite timeout

[Bartosz Golaszewski]

On Tue, May 23, 2023 at 3:18 PM Nicolas Frattaroli
<frattaroli.nicolas@gmail.com> wrote:
>
> On Dienstag, 23. Mai 2023 15:04:54 CEST Bartosz Golaszewski wrote:
> > On Tue, May 23, 2023 at 2:38 PM Nicolas Frattaroli
> > <frattaroli.nicolas@gmail.com> wrote:
> > >
> > > On Dienstag, 23. Mai 2023 14:33:12 CEST Bartosz Golaszewski wrote:
> > > >
> > > > ...
> > > >
> > > > As has been said elsewhere - the pythonic way is to interpret None as
> > > > indefinite timeout. It's just that it would change the current
> > > > behavior. The question is - should we interpret the current behavior
> > > > as "undefined" and change it, or "defined but not documented" and
> > > > consider it part of the API.
> > > >
> > > > Bart
> > > >
> > >
> > > As an alternate suggestion, we could change the default function argument
> > > to 0.0 and remove the None -> 0 code. That way, people who were calling
> > > the function with no arguments still get the same behaviour, and the
> > > only break is for users who explicitly passed None.
> > >
> >
> > Honestly, if we were to change the behavior, then I'd prefer to do it
> > right and not use any half-measures.
> >
> > Bart
> >
>
> That's fair, I don't mind either way. I don't think the behavioural
> change would be that big of an issue either way; the obviousness of a
> wait on events function that returns immediately by default is
> debatable in my eyes, as the only use for using it like that I could
> ever come up with is to peek at whether there are any events queued
> up based on its return value.
>
> What I do think is a bad solution now is the float("inf") stuff.
> While it's fun to assign meaning to special floating point values,
> I think it strictly makes the API worse for the sake of stability.
>

For sure, I just preferred it in case of keeping the old behavior. Let
me send out a patch changing the behavior.

Bart