Skip to content

Add type hints #8

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 8 commits into from
Oct 14, 2021
Merged

Add type hints #8

merged 8 commits into from
Oct 14, 2021

Conversation

jsymons
Copy link
Contributor

@jsymons jsymons commented Oct 7, 2021

Adds type hints to the following files per #7 :

  • adafruit_is31fl3741/__init__.py
  • adafruit_is31fl3741/issi_evb.py
  • adafruit_is31fl3741/adafruit_rgbmatrixqt.py
  • adafruit_is31fl3741/adafruit_ledglasses.py

@jsymons
Copy link
Contributor Author

jsymons commented Oct 7, 2021

The particular part of the CI failing is because I've added type hints for a PIL Image where it's expecting one, but Pillow is not part of the requirements.txt.

Would you rather I update the requirements.txt to require Pillow or find some typing workaround?

FoamyGuy
FoamyGuy requested changes Oct 7, 2021
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 @jsymons

@FoamyGuy
Copy link
Contributor

FoamyGuy commented Oct 7, 2021

The particular part of the CI failing is because I've added type hints for a PIL Image where it's expecting one, but Pillow is not part of the requirements.txt.

Would you rather I update the requirements.txt to require Pillow or find some typing workaround?

I think we can add Pillow to the requirements.txt it would be needed in order to use that function if I understand correctly.

@FoamyGuy
Copy link
Contributor

FoamyGuy commented Oct 7, 2021

I will look this over later on tonight.

@FoamyGuy FoamyGuy self-requested a review October 7, 2021 13:57
FoamyGuy
FoamyGuy reviewed Oct 7, 2021
View reviewed changes
Copy link
Contributor

@FoamyGuy FoamyGuy left a comment •
edited
Loading

Choose a reason for hiding this comment

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

The changes from this PR look good to me. However I do not have the hardware to test with.

It needs merged from main to resolve some conflicts now though. Do you know how to do that @jsymons?

@jsymons
Copy link
Contributor Author

jsymons commented Oct 7, 2021

Yeah, I'll get to it later tonight.

jsymons
jsymons commented Oct 8, 2021
View reviewed changes
@jsymons
Copy link
Contributor Author

jsymons commented Oct 8, 2021

Conflicts have been resolved now @FoamyGuy

gamblor21
gamblor21 approved these changes Oct 13, 2021
View reviewed changes
Copy link
Member

@gamblor21 gamblor21 left a comment

Choose a reason for hiding this comment

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

Looks good to me now! @FoamyGuy I do not think either of us have all the h/w to test all of these we may want to find out if it is worth just merging them as they are passing CI and none of the changes should affect them running?

FoamyGuy
FoamyGuy approved these changes Oct 14, 2021
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.

Looks good to me. Thanks for working on this @jsymons

@FoamyGuy FoamyGuy merged commit 3cabc01 into adafruit:main Oct 14, 2021
@FoamyGuy FoamyGuy mentioned this pull request Oct 14, 2021
Closed
20 tasks
adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Oct 15, 2021
@adafruit-adabot
Automated update by Adabot (adafruit/adabot@b81efd7)
e031f83 
Updating https://github.com/adafruit/Adafruit_CircuitPython_IS31FL3741 to 1.1.4 from 1.1.3:
  > Merge pull request adafruit/Adafruit_CircuitPython_IS31FL3741#8 from jsymons/typing

Updating https://github.com/adafruit/Adafruit_CircuitPython_OV5640 to 1.0.1 from 1.0.0:
  > Merge pull request adafruit/Adafruit_CircuitPython_OV5640#7 from adafruit/fix-effects-modes
  > Merge pull request adafruit/Adafruit_CircuitPython_OV5640#5 from adafruit/boost-pixel-clock

Updating https://github.com/adafruit/Adafruit_CircuitPython_Seesaw to 1.10.1 from 1.10.0:
  > Merge pull request adafruit/Adafruit_CircuitPython_seesaw#86 from kattni/simpletest
@jsymons jsymons mentioned this pull request Oct 15, 2021
Merged
@jsymons jsymons deleted the typing branch October 15, 2021 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gamblor21 gamblor21 gamblor21 approved these changes

@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.
3 participants
@jsymons @FoamyGuy @gamblor21