add type hints, restrict return type of monitored callable

[awordforthat]

This is part of the effort to add type hinting to existing libraries.

As part of this, I'm suggesting a minor modification to the library. As written, you can monitor a function as if it were a digital input. This isn't a problem, theoretically. However, the library allows you to request the current value of the debounced input, which either returns a bool (if you're debouncing a DigitalInOut) or literally anything if you're using a function. I'm suggesting that debounced function should be restricted to returning a bool so the behavior of this debouncer is more predictable.

Suggestions/comments welcome!

[tekktrik]

Good use of Callable! A few changes and requests for a second look from @FoamyGuy!

[tekktrik]

Looking for a second opinion again from FoamyGuy

[Neradoc]

Note that, despite the description, io_or_predicate doesn't take a DigitalInOut, it takes anything that has a value property. That includes touchio.TouchIn, adafruit_seesaw.digitalio.DigitalIO...
So that would require a Protocol. Something like that:

class InputOutputType(Protocol):
    value: bool

[tekktrik]

@Neradoc oh very good point about there being other classes that make use of value! In that case I definitely agree it's worth keeping as is!

@awordforthat, you can add a Protocol (how we designate "duck typing") inside of the typing try/except block with the protocol @Neradoc mentions above. It should look very similar to how it's done here with the Pixel class: https://github.com/tekktrik/Adafruit_CircuitPython_ESP_ATcontrol/blob/d3ba70ac87841bc3ac5fba7e1f157212614c9d39/adafruit_espatcontrol/adafruit_espatcontrol_wifimanager.py#L28~~

Note that Python 3.7 has the Protocol class in the typing_extensions module, so we actually use another nested try/except block above it to import Protocol from either the typing module if 3.8+ or typing_extensions if 3.7.

[FoamyGuy]

Thanks for working on this @awordforthat

A few things tekktrik noted I think we do want to do. The int() calls I am open to including but also open to thoughts from other folks and definitely think we want to test it to make sure no unintended effects.

As for restricting the function to one returning a boolean, I'm for specifying it in the docstring and types (although I didn't know it was possible to do in the types, but that looks like what you've done for the Callable neat usage of that). But I don't think we should put any further technical restriction on it necessarily like checking for it and raising a exception.

I think it's good to point folks toward using functions that return bools, but if someone has a different use case that doesn't happen to return bool but still does what they want then I think we should allow that behavior.

[echarles-dev]

Thanks all for your input! I agree with everyone that we should not restrict the type of value that this can monitor. I'll get to work on cleaning up the rest of the types :)

[echarles-dev]

Note that, despite the description, io_or_predicate doesn't take a DigitalInOut, it takes anything that has a value property. That includes touchio.TouchIn, adafruit_seesaw.digitalio.DigitalIO... So that would require a Protocol. Something like that:

class InputOutputType(Protocol):
    value: bool

@Neradoc value here could theoretically be any type. Would you prefer that it's specified as bool?

[Neradoc]

I don't know, it could be anything that converts to bool I guess.
This code seems to indicate we expect bool from the predicate, since the alternate argument is converted to bool.

Adafruit_CircuitPython_Debouncer/adafruit_debouncer.py

Lines 80 to 83 in e7a969f

	if new_state is None:
	current_state = self.function()
	else:
	current_state = bool(new_state)

Also there is analogio.AnalogIn.value that is an int, so not the best name for the protocol in general.

[echarles-dev]

@Neradoc ah shoot you're right. All of the get/set/toggle methods also "expect" a boolean, although they'd work with some other values. Since it's only a hint and not a restriction, I guess I'm ok typing it as bool

[tekktrik]

I'm torn here. bool is a subclass of int so int would also be acceptable to capture the behavior. I think this is also really more of the intended behavior of this function, and the only thing we probably want to "promise" will work in this and future implementations.

However, if we want to type for "truthiness", I think this is one of the rare times we can type the return as object since any object in Python can be evaluated for truthiness. The risk is that if we ever do anything else that a base object couldn't do in the future (say addition or something 🤷‍♂️), it's now a breaking change technically.

My vote is for the former, but am curious what others think.

[FoamyGuy]

@echarles-dev are you still working on this one to make the discussed changes? I will try to wrap it up with these changes next week if you're not, or don't have time between now and then.

[tekktrik]

Update: adafruit/Adafruit_CircuitPython_Typing#18 defined a Protocol for classes defining a value attribute, so that can be used here.

[FoamyGuy]

I believe I made all of the changes requested in prior feedback with the latest commit.

@tekktrik if you have a chance want to take a another look over this one?

[tekktrik]

Sure thing! I can review this tonight.

[tekktrik]

Looks good to me! Thanks y'all!