Skip to content

Improve error checking to Matrix parameters #131

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.

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

Merged
merged 13 commits into from
Apr 22, 2022

Conversation

mutesplash
Copy link
Contributor

@mutesplash mutesplash commented Mar 21, 2022

De-duplicates code and also allows set_pixels to accept (str,int) as pixel specifications

Allow set_pixels() to defer display updates like set_pixel() does

Also, remove the exception in Device.off() for the Matrix by overriding it

@mutesplash
Copy link
Contributor Author

Updated!

@mutesplash
Copy link
Contributor Author

I guess I better get flake8 installed if I want this merged.

@chrisruk
Copy link
Contributor

Hey, sorry about that, I've just been working on linting the code, you'll need -

flake8
flake8-pylint

Then you can do: 'PYTHONPATH=. flake8 .' from the main part of the repo.

@chrisruk
Copy link
Contributor

This now fails to run the example in docs for me.

With:

python3.9 docs/buildhat/matrix.py 
Traceback (most recent call last):
  File "/home/pi/Repositories/python-build-hat/docs/buildhat/matrix.py", line 8, in <module>
    matrix.clear(("red", 10))
  File "/home/pi/Repositories/python-build-hat/buildhat/matrix.py", line 121, in clear
    color = Matrix.normalize_pixel(pixel)  # pylint: disable=too-many-function-args
TypeError: normalize_pixel() takes 1 positional argument but 2 were given

Also at the moment I'm not really using the type annotation feature.

I think staticmethod should be used rather than classmethod.

@mutesplash
Copy link
Contributor Author

Ugh, these tuples...

I put the type annotations in to make the linter be quiet. If you'd rather use the # directives, I'll change it back. I'll also use the staticmethod.

honestly, I didn't like them to begin with
@mutesplash
Copy link
Contributor Author

Ooookay, turned out those didn't help the linter anyway! It should work now?

@chrisruk chrisruk merged commit 9130da5 into RaspberryPiFoundation:main Apr 22, 2022
@mutesplash mutesplash deleted the patch-2 branch April 22, 2022 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants