Skip to content

Example with image rendering functionality #51

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 12 commits into from
Aug 27, 2023
Merged

Conversation

jim-mckeown
Copy link
Contributor

Example from modified fingerprint_simpletest.py adding image viewing and saving functionality using matplotlib and numpy. This has been tested on a Linux PC but should work under Windows or MacOS.

Example from modified fingerprint_simpletest.py adding image viewing and saving functionality using matplotlib and numpy. This has been tested on a Linux PC but should work under Windows or MacOS.
@tekktrik
Copy link
Member

Looks like this is failing the GitHub Actions CI. You can use pre-commit to help find and fix the issues it's having. You can find information on installing and using pre-commit here: https://learn.adafruit.com/creating-and-sharing-a-circuitpython-library/check-your-code

@tekktrik tekktrik requested a review from a team April 27, 2023 20:36
Fixed import order error. pyplot changed to non-blocking to run properly from the command line.
@jim-mckeown
Copy link
Contributor Author

I changed the import order and reformatted with black. Also added non-blocking mode in matplotlib.pyplot so the code will run properly from the command line.

Copy link
Member

@tekktrik tekktrik 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 the submission! I added some feedback, let me know if I can expand on anything. I can see if anyone can test this too, which would be good if we can arrange.

@@ -0,0 +1,254 @@
# SPDX-FileCopyrightText: 2021 ladyada for Adafruit Industries
Copy link
Member

Choose a reason for hiding this comment

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

I would change this to your name, as well as remove the credit to Adafruit Industries:

Suggested change
# SPDX-FileCopyrightText: 2021 ladyada for Adafruit Industries
# SPDX-FileCopyrightText: 2023 Jim McKeown

Comment on lines 4 to 5
# Added 'View Print' and 'Preview and Find Print' functions to
# example code fingerprint_simpletest.py
Copy link
Member

Choose a reason for hiding this comment

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

We typically use triple quotes for docstrings, but I don't think this is a gating change. More importantly, I would specify that this example only works on single board computers with the use of Blinka, our CircuitPython compatibility layer.

Comment on lines +12 to +14
import numpy as np
from matplotlib import pyplot as plt
import serial
Copy link
Member

Choose a reason for hiding this comment

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

Since these are used in examples, we need to add these as dependencies to optional_requirements.txt!

@jim-mckeown
Copy link
Contributor Author

Please have a look at the latest changes and let me know if any others are needed or would be helpful. Thanks.

Copy link
Member

@tekktrik tekktrik 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 the changes! Just a few small tweaks, but we'll see if I or someone else can test this after these get updated!

@jim-mckeown
Copy link
Contributor Author

Please have a look at the latest changes.

Just one thought about pyserial. Since it is already in requirements.txt, does it also need to be in optional_requirements.txt?

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.

All requested changes have been addressed with newer commits.

@FoamyGuy FoamyGuy merged commit ccec412 into adafruit:main Aug 27, 2023
adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Aug 28, 2023
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.

3 participants