search

adafruit / Adafruit_CircuitPython_EPD

e paper driver for circuit python
MIT License
40 stars 19 forks source link

Added requested type annotations to epd.py. #65

Closed sdomoszlai13 closed 1 year ago

sdomoszlai13 commented 1 year ago

Please let me know if something's wrong with the annotations.

sdomoszlai13 commented 1 year ago

The Build CI test fails because of a pylint error, which should be ignored if I'm not mistaken (there's the '# pylint: disable=too-many-arguments' directive in the code in those lines). Can you tell me what's wrong with my commit?

Also, excuse me for closing the other pull request. I just meant to rename that branch to a better name. @FoamyGuy

FoamyGuy commented 1 year ago

with regards to the pylint error. You can add too-many-arguments to the disabled checks on the class declaration line:

It's already got a few different ones but this one can be added at the end of the list like this:

class Adafruit_EPD:  # pylint: disable=too-many-instance-attributes, too-many-public-methods, too-many-arguments

With it disabled from there it won't need to be included on any of the individual functions and it will allow it to pass the check

sdomoszlai13 commented 1 year ago

Thanks for your valuable suggestions @FoamyGuy ! I implemented them in this branch and merged the others in this one. Now you can review my changes made in the other files here as well.

Locally, the pylint test completes without errors, but in GitHub it complains about the type Image, which is part of the PIL package:

File "/home/runner/work/Adafruit_CircuitPython_EPD/Adafruit_CircuitPython_EPD/adafruit_epd/epd.py", line 399, in Adafruit_EPD def image(self, image: Image) -> None: ^^^^^ NameError: name 'Image' is not defined

I have the import command inside the try block:

from PIL.Image import Image

Any suggestions?

sdomoszlai13 commented 1 year ago

Requested changes implemented. Thanks for your support @FoamyGuy ! Should I close the other pull requests, since I merged the corresponding branches to this branch?

FoamyGuy commented 1 year ago

@sdomoszlai13 Thanks again for working on this!

Yes you can close the other PRs now since everything is included in this one.

FoamyGuy commented 1 year ago

Oh, I noticed after I left the prior comment that Github seems to have figured it out automatically and closed those other ones so you shouldn't have to do anything on your end actually.