Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Adding annotations #37

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Adding annotations #37

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c84f7ce
Adding type-hint annotations
andrib-94 Apr 25, 2023
2047a40
Adding type-hint annotations
andrib-94 Apr 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
185 changes: 104 additions & 81 deletions adafruit_button.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,12 @@

"""

try:
from typing import Optional, Union, List, Any
from fontio import FontProtocol
except ImportError:
pass

from micropython import const
import displayio
from adafruit_display_text.label import Label
Expand All @@ -31,7 +37,7 @@
__repo__ = "https://github.com/adafruit/Adafruit_CircuitPython_Display_Button.git"


def _check_color(color):
def _check_color(color: Optional[Union[int, tuple[int, int, int]]]) -> Optional[int]:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have an Alias that covers these types of Color arguments. But it's in a file that associates it with leds: https://github.com/adafruit/Adafruit_CircuitPython_Typing/blob/bd358db434d2e2c155c897d7f3c9674b04a3b67f/circuitpython_typing/led.py#L18

@tekktrik do you think it's worth making a slightly more generic Alias for Color inside of circuitpython_typing that can be used in other libraries for hex / tuple colors? Many of our libraries expect both in the context of displayio as well as LEDs.

Or alternatively is it problematic to use the typing from inside of led.py here even though this context isn't dealing with LEDs?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, let me know when you decide on the way forward with Color implementation and I am happy to import it from appropriate module : )

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've put a note to discuss this in the weeds for the upcoming weekly meeting.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think having multiple aliases is a good idea, sorry for the late response!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could always change up the way that colors are defined, potentially moving them out of led.py and into a new file like color.py.

# if a tuple is supplied, convert it to a RGB number
if isinstance(color, tuple):
r, g, b = color
Expand Down Expand Up @@ -59,12 +65,68 @@ class Button(displayio.Group):
:param selected_outline: Inverts the outline color.
:param selected_label: Inverts the label color.
"""
RECT: int = const(0)
ROUNDRECT: int = const(1)
SHADOWRECT: int = const(2)
SHADOWROUNDRECT: int = const(3)

def __init__(
self,
*,
x: int,
y: int,
width: int,
height: int,
name: Optional[str] = None,
style: int = RECT,
fill_color: Optional[int] = 0xFFFFFF,
outline_color: Optional[int] = 0x0,
andrib-94 marked this conversation as resolved.
Show resolved Hide resolved
label: Optional[Label] = None,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

label is expecting a string type. It will eventually put that string into a Label object, but when passed in here it should be a string I believe.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, so here is where I was struggling during Sprint. When label is not Label object, both property and property setter for it throw errors, as we utilize Label object's arguments (.text, .bounding_box etc.) on variable we hinted as string.

Is there any way to work-around this issue, so mypy does not throw errors, when checking?

Thanks!

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@perkin-borowa I think the type hinter may be confused by this line in the init function:

self._label = label

This is setting the argument label onto self._label which is technically mixing types. The rest of the code is written to assume that self._label will be a Label object.

It never caused any other trouble because the last line of the init function ends up superceding this one that is erroneously setting self._label to the string argument.

self.label = label

That line calls the setter which is expecting the text to get passed into it.

Try removing

self._label = label

And see if mypy still reports any errors about Label object vs. string for the argument.

label_font: Optional[FontProtocol] = None,
label_color: Optional[int] = 0x0,
FoamyGuy marked this conversation as resolved.
Show resolved Hide resolved
selected_fill: Optional[Union[int, tuple[int, int, int]]] = None,
selected_outline: Optional[Union[int, tuple[int, int, int]]] = None,
selected_label: Optional[Union[int, tuple[int, int, int]]] = None
):
super().__init__(x=x, y=y)
self.x = x
self.y = y
self._width = width
self._height = height
self._font = label_font
self._selected = False
self.name = name
self._label = label
self.body = self.fill = self.shadow = None
self.style = style

self._fill_color = _check_color(fill_color)
self._outline_color = _check_color(outline_color)
self._label_color = label_color
self._label_font = label_font
# Selecting inverts the button colors!
self._selected_fill = _check_color(selected_fill)
self._selected_outline = _check_color(selected_outline)
self._selected_label = _check_color(selected_label)

if self.selected_fill is None and fill_color is not None:
assert self._fill_color is not None
self.selected_fill = (~self._fill_color) & 0xFFFFFF
if self.selected_outline is None and outline_color is not None:
assert self._outline_color is not None
self.selected_outline = (~self._outline_color) & 0xFFFFFF

self._create_body()
if self.body:
self.append(self.body)

self.label = label

def _empty_self_group(self):
def _empty_self_group(self) -> None:
while len(self) > 0:
self.pop()

def _create_body(self):
def _create_body(self) -> None:
if (self.outline_color is not None) or (self.fill_color is not None):
if self.style == Button.RECT:
self.body = Rect(
Expand Down Expand Up @@ -118,68 +180,14 @@ def _create_body(self):
if self.shadow:
self.append(self.shadow)

RECT = const(0)
ROUNDRECT = const(1)
SHADOWRECT = const(2)
SHADOWROUNDRECT = const(3)

def __init__(
self,
*,
x,
y,
width,
height,
name=None,
style=RECT,
fill_color=0xFFFFFF,
outline_color=0x0,
label=None,
label_font=None,
label_color=0x0,
selected_fill=None,
selected_outline=None,
selected_label=None
):
super().__init__(x=x, y=y)
self.x = x
self.y = y
self._width = width
self._height = height
self._font = label_font
self._selected = False
self.name = name
self._label = label
self.body = self.fill = self.shadow = None
self.style = style

self._fill_color = _check_color(fill_color)
self._outline_color = _check_color(outline_color)
self._label_color = label_color
self._label_font = label_font
# Selecting inverts the button colors!
self._selected_fill = _check_color(selected_fill)
self._selected_outline = _check_color(selected_outline)
self._selected_label = _check_color(selected_label)

if self.selected_fill is None and fill_color is not None:
self.selected_fill = (~self._fill_color) & 0xFFFFFF
if self.selected_outline is None and outline_color is not None:
self.selected_outline = (~self._outline_color) & 0xFFFFFF

self._create_body()
if self.body:
self.append(self.body)

self.label = label

@property
def label(self):
def label(self) -> Optional[Union[Any, str]]:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure we need Any here for the return type. Are there cases where the return will be something other than a string or None?

"""The text label of the button"""
assert self._label is not None
return self._label.text

@label.setter
def label(self, newtext):
def label(self, newtext: Optional[str]) -> None:
if self._label and self and (self[-1] == self._label):
self.pop()

Expand Down Expand Up @@ -208,12 +216,12 @@ def label(self, newtext):
self.selected_label = (~self._label_color) & 0xFFFFFF

@property
def selected(self):
def selected(self) -> bool:
"""Selected inverts the colors."""
return self._selected

@selected.setter
def selected(self, value):
def selected(self, value: bool) -> None:
if value == self._selected:
return # bail now, nothing more to do
self._selected = value
Expand All @@ -233,7 +241,7 @@ def selected(self, value):
self._label.color = new_label

@property
def group(self):
def group(self) -> "Button":
"""Return self for compatibility with old API."""
print(
"Warning: The group property is being deprecated. "
Expand All @@ -242,7 +250,7 @@ def group(self):
)
return self

def contains(self, point):
def contains(self, point: List[int]) -> bool:
"""Used to determine if a point is contained within a button. For example,
``button.contains(touch)`` where ``touch`` is the touch point on the screen will allow for
determining that a button has been touched.
Expand All @@ -252,75 +260,90 @@ def contains(self, point):
)

@property
def fill_color(self):
def fill_color(self) -> Optional[int]:
"""The fill color of the button body"""
return self._fill_color

@fill_color.setter
def fill_color(self, new_color):
def fill_color(self, new_color: Optional[Union[int, tuple[int, int, int]]]) -> None:
self._fill_color = _check_color(new_color)
if not self.selected:
assert self.body is not None
self.body.fill = self._fill_color

@property
def outline_color(self):
def outline_color(self) -> Optional[int]:
"""The outline color of the button body"""
return self._outline_color

@outline_color.setter
def outline_color(self, new_color):
def outline_color(
self, new_color: Optional[Union[int, tuple[int, int, int]]]
) -> None:
self._outline_color = _check_color(new_color)
if not self.selected:
assert self.body is not None
self.body.outline = self._outline_color

@property
def selected_fill(self):
def selected_fill(self) -> Optional[int]:
"""The fill color of the button body when selected"""
return self._selected_fill

@selected_fill.setter
def selected_fill(self, new_color):
def selected_fill(
self, new_color: Optional[Union[int, tuple[int, int, int]]]
) -> None:
self._selected_fill = _check_color(new_color)
if self.selected:
assert self.body is not None
self.body.fill = self._selected_fill

@property
def selected_outline(self):
def selected_outline(self) -> Optional[int]:
"""The outline color of the button body when selected"""
return self._selected_outline

@selected_outline.setter
def selected_outline(self, new_color):
def selected_outline(
self, new_color: Optional[Union[int, tuple[int, int, int]]]
) -> None:
self._selected_outline = _check_color(new_color)
if self.selected:
assert self.body is not None
self.body.outline = self._selected_outline

@property
def selected_label(self):
def selected_label(self) -> Optional[int]:
"""The font color of the button when selected"""
return self._selected_label

@selected_label.setter
def selected_label(self, new_color):
def selected_label(
self, new_color: Optional[Union[int, tuple[int, int, int]]]
) -> None:
self._selected_label = _check_color(new_color)

@property
def label_color(self):
def label_color(self) -> Optional[int]:
"""The font color of the button"""
return self._label_color

@label_color.setter
def label_color(self, new_color):
def label_color(
self, new_color: Optional[Union[int, tuple[int, int, int]]]
) -> None:
self._label_color = _check_color(new_color)
assert self._label is not None
self._label.color = self._label_color

@property
def width(self):
def width(self) -> int:
"""The width of the button"""
return self._width

@width.setter
def width(self, new_width):
def width(self, new_width: int) -> None:
self._width = new_width
self._empty_self_group()
self._create_body()
Expand All @@ -329,20 +352,20 @@ def width(self, new_width):
self.label = self.label

@property
def height(self):
def height(self) -> int:
"""The height of the button"""
return self._height

@height.setter
def height(self, new_height):
def height(self, new_height: int) -> None:
self._height = new_height
self._empty_self_group()
self._create_body()
if self.body:
self.append(self.body)
self.label = self.label

def resize(self, new_width, new_height):
def resize(self, new_width: int, new_height: int) -> None:
"""Resize the button to the new width and height given
:param new_width int the desired width
:param new_height int the desired height
Expand Down