Skip to content

initial typing support, mypy passes #111

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
Merged
merged 3 commits into from
Apr 27, 2023
Merged

initial typing support, mypy passes #111

merged 3 commits into from
Apr 27, 2023

Conversation

matt-land
Copy link
Contributor

@matt-land matt-land commented Apr 24, 2023 •
edited
Loading

This adds support for #96

Changes

  • adds all types
  • modifies rgb.py/color565() to not change types of the r value from tuple to int (mypy assignment and a ton of other errors)
  • adds -> read and -> write methods to Display class to (7x mypy attr-defined errors.) They are implemented in the child DisplaySPI class, and if this wasn't Circuitpython, I'm sure it would use abc.Meta. Feedback requested.
  • adds a RuntimeError if DisplaySPI->reset() is called, but the reset pin was not defined during init (Mypy attr )
  • adds pre-commit hook to the project to run mypy
  • trivial optimization to SSD1351, removed a reference for setting baudrate

Testing

pipenv shell
pip install mypy
pip install requirements.txt

mypy adafruit_rgb_display

Success: no issues found in 10 source files

@matt-land matt-land force-pushed the add-types-pycon-2023 branch 2 times, most recently from f0ff6d6 to e758c6b Compare April 24, 2023 22:07
@matt-land matt-land changed the title WIP initial typing support, mypy passes initial typing support, mypy passes Apr 24, 2023
@tekktrik tekktrik requested a review from a team April 24, 2023 22:18
@matt-land matt-land force-pushed the add-types-pycon-2023 branch from e758c6b to 5780e9b Compare April 24, 2023 22:26
@matt-land
initial typing support, mypy passes
79eb6fc 
add missing copyright statements to .pre-commit-config.yaml, gitignore
@matt-land matt-land force-pushed the add-types-pycon-2023 branch from 5780e9b to 79eb6fc Compare April 24, 2023 22:35
FoamyGuy
FoamyGuy requested changes Apr 24, 2023
View reviewed changes
Copy link
Contributor

@FoamyGuy FoamyGuy left a comment

Choose a reason for hiding this comment

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

Thanks for working on this @matt-land!

I had a look over this, majority is looking good to me. I had a few questions and suggested changes that I've noted in comments at the relevant sections.

This was referenced Apr 25, 2023
Closed
Merged
FoamyGuy
FoamyGuy reviewed Apr 25, 2023
View reviewed changes
Copy link
Contributor

@FoamyGuy FoamyGuy left a comment

Choose a reason for hiding this comment

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

The latest version is looking pretty much good to me.

One more tweak that I found taking another look over it this morning is noted below.

I've opened up a PR in the typing repo to add the requisite attributes on our PIL.Image stand-in.

adafruit_rgb_display/rgb.py Outdated
"""Dummy switch_to_input method"""

@property
def value(self):
def value(self) -> digitalio.DigitalInOut:
Copy link
Contributor

Choose a reason for hiding this comment

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

I missed this on the first pass, I think value would be boolean True / False.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed. thanks for the quick reviews

FoamyGuy
FoamyGuy approved these changes Apr 27, 2023
View reviewed changes
Copy link
Contributor

@FoamyGuy FoamyGuy left a comment

Choose a reason for hiding this comment

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

Latest version looks good to me.

Thanks again for your contributions across libraries @matt-land!

nothing in here should really change any functionality, but I did give it a quick test to double check on a Feather S2 TFT with the built-in display using the simpletest from this repo adapted to the correct pins. All is behaving as expected.

@FoamyGuy FoamyGuy merged commit 6ca6455 into adafruit:main Apr 27, 2023
@FoamyGuy FoamyGuy mentioned this pull request Apr 27, 2023
Closed
34 tasks
adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Apr 27, 2023
@adafruit-adabot
Automated update by Adabot (adafruit/adabot@a52bbea)
700120a 
Updating https://github.com/adafruit/Adafruit_CircuitPython_RGB_Display to 3.11.1 from 3.11.0:
  > Merge pull request adafruit/Adafruit_CircuitPython_RGB_Display#111 from matt-land/add-types-pycon-2023

Updating https://github.com/adafruit/Adafruit_CircuitPython_Ducky to 1.1.0 from 1.0.11:
  > Merge pull request adafruit/Adafruit_CircuitPython_Ducky#8 from pstray/no_sleep
  > Merge pull request adafruit/Adafruit_CircuitPython_Ducky#11 from FoamyGuy/ramsrq_fixes
  > Add upload url to release action
  > Add .venv to .gitignore

Updating https://github.com/adafruit/Adafruit_CircuitPython_Bundle/circuitpython_library_list.md to NA from NA:
  > Updated download stats for the libraries
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FoamyGuy FoamyGuy FoamyGuy approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@matt-land @FoamyGuy