-
Notifications
You must be signed in to change notification settings - Fork 18
Function that clears the _spark_list list #32
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this addition and for this PR. Just a few minor things. As currently written, this PR clears all the data values but leaves all the Line
elements intact. Please add the following to clear all the lines (snipped from the update function).
for _ in range(len(self)): # remove all items from the current group
self.pop()
also, I suggest adding a comment to the function so the documentation will display your newly-added Sparkline.clear
function. Something like:
def clear_values(self):
"""Clears all values and lines from the sparkline"""
self._spark_list = []
Finally, something caused the PR to fail the automated checks, maybe something minor with formatting or trailing spaces. Please run pylint and black and resubmit. Here's the instructions on running pylint and black.
Many thanks for your contribution, glad to see that the sparkline feature is useful!
Oh yep, in my code i called update right after so I didn't notice will update the PR! Thank you! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed the file and looks good to me, but didn't verify on hardware. Seems to be failing the black
automated checkout.
@Lukas1337 Just checking in to see if you have time to update and submit this one. Looks really close to being ready, just a few tweaks to get it to pass all the checks. Reach out if you have questions or need help. |
Hey, thanks for the reminder I almost forgot about this but yes I don't really understand how to get black working. I attempted to run it but didn't notice any immediate differences so I'm probably doing something wrong. |
Oh thanks!!
@Lukas1337 thanks for following up on this! I think you got it correctly. Black can be very picky even about small things. In this case it removed a few extra spaces. So without some kind of fancy diff highlight tool it would have been near impossible to spot. |
It looks like it got caught again on the commit from the merge. Sorry for all of the back and forth @Lukas1337. I think it needs to have black run once more and then a commit after that. It looks like you've allowed project maintainers permission to push to this branch. If you'd rather not mess with it any more, no worries, we can get it finished up for you, let us know. Thank again for your work on this enhancement! |
That would be great, feel free to finish it up whenever you've got time! |
There was a problem hiding this 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.
I tested this successfully with
Adafruit CircuitPython 6.2.0-beta.4 on 2021-03-18; Adafruit PyPortal with samd51j20
Thanks for this enhancement @Lukas1337!
Thanks @Lukas1337 and @FoamyGuy for closing this one out! |
Updating https://github.com/adafruit/Adafruit_CircuitPython_Display_Shapes to 2.0.8 from 2.0.7: > Merge pull request adafruit/Adafruit_CircuitPython_Display_Shapes#32 from Lukas1337/master > "Increase duplicate code check threshold " Updating https://github.com/adafruit/Adafruit_CircuitPython_DisplayIO_Layout to 1.7.0 from 1.6.0: > Merge pull request adafruit/Adafruit_CircuitPython_DisplayIO_Layout#27 from kmatch98/annotation_PR > Merge pull request adafruit/Adafruit_CircuitPython_DisplayIO_Layout#26 from kmatch98/dial_rotation Updating https://github.com/adafruit/Adafruit_CircuitPython_FeatherWing to 1.14.1 from 1.14.0: > Merge pull request adafruit/Adafruit_CircuitPython_FeatherWing#69 from jfurcean/expose_cs_pins
Tested on my pyportal using circuitpython 6.1.0, seems to work as expected. Fairly simple change but I find it quite useful being able to clear a sparkline without reinstating it.