Skip to content

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

Merged
merged 6 commits into from
Mar 25, 2021

Conversation

eriklukas
Copy link

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.

Copy link

@kmatch98 kmatch98 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 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!

@eriklukas
Copy link
Author

Oh yep, in my code i called update right after so I didn't notice will update the PR! Thank you!

Copy link

@kmatch98 kmatch98 left a 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.

@kmatch98
Copy link

@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.

@eriklukas
Copy link
Author

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.

@FoamyGuy
Copy link

@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.

@FoamyGuy
Copy link

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!

@eriklukas
Copy link
Author

That would be great, feel free to finish it up whenever you've got time!

Copy link

@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.

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!

@FoamyGuy FoamyGuy merged commit 5d8bffe into adafruit:master Mar 25, 2021
@kmatch98
Copy link

Thanks @Lukas1337 and @FoamyGuy for closing this one out!

adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Mar 25, 2021
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
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