-
Notifications
You must be signed in to change notification settings - Fork 60
Enhanced parsing of timestamp_utc #76
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
I guess the tests are failing because of the changed type. Before trying to change these, some feedback would be appreciated. |
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.
The fix itself looks good to me. Since this library has pretty good tests, I think you should add a new test for the specific date you found failed with the float code.
So, copy one of the test functions that you had to modify to use a string and make it use the date/time that fails with floats. This will prevent anyone from changing it back to float because it won't pass. :-)
The tests actually already fails when trying with floats or ints:
Because you can't slice ints and floats. |
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.
I see your point. What I should have said was that when bugs like this are found, it's good practice to add a new test that fails before fixing it. Then, you know you fixed the issue.
However, this may not work since it is a MicroPython VM bug, not a Python one. So, I'll approve and merge. Thanks!
@tannewt Thank you for explaining. I'm not familiar with best pactice for writing tests, but i will keep your advice in mind for next time ! :-) |
Updating https://github.com/adafruit/Adafruit_CircuitPython_GPS to 3.9.6 from 3.9.5: > Merge pull request adafruit/Adafruit_CircuitPython_GPS#76 from mrdalgaard/main > update rtd py version Updating https://github.com/adafruit/Adafruit_CircuitPython_PyPortal to 6.1.0 from 6.0.1: > Merge pull request adafruit/Adafruit_CircuitPython_PyPortal#116 from joebaird/main > update rtd py version Updating https://github.com/adafruit/Adafruit_CircuitPython_Seesaw to 1.10.4 from 1.10.3: > Merge pull request adafruit/Adafruit_CircuitPython_seesaw#90 from todbot/main > update rtd py version Updating https://github.com/adafruit/Adafruit_CircuitPython_Display_Shapes to 2.4.1 from 2.4.0: > Merge pull request adafruit/Adafruit_CircuitPython_Display_Shapes#45 from dhalbert/make-package Updating https://github.com/adafruit/Adafruit_CircuitPython_RTTTL to 2.4.10 from 2.4.9: > Merge pull request adafruit/Adafruit_CircuitPython_RTTTL#29 from FoamyGuy/pyportal_fix Updating https://github.com/adafruit/Adafruit_CircuitPython_TinyLoRa to 2.2.5 from 2.2.4: > update rtd py version
Closes #75
Uses string parsing instead of math for generating timestamp_utc.
Avoids imprecise usage of float.
Sorry if i messed this up, its my first pull request.
Tested on Adafruit Feather Bluefruit Sense, and CP7.2.0-alpha.0.
I do not have any low-memory MCUs to test on such as the M0