-
Notifications
You must be signed in to change notification settings - Fork 16
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
DRAFT added support for centiseconds in the TimeElementsWithFractionOfSeconds #282
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #282 +/- ##
==========================================
+ Coverage 96.20% 96.67% +0.47%
==========================================
Files 24 24
Lines 2343 2375 +32
==========================================
+ Hits 2254 2296 +42
+ Misses 89 79 -10 ☔ View full report in Codecov by Sentry. |
tests/time_elements.py
Outdated
|
||
expected_time_elements_tuple = (2010, 8, 12, 20, 6, 31) | ||
time_elements_object = time_elements.TimeElementsWithFractionOfSecond( | ||
fraction_of_second=decimal.Decimal(0.8742), |
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.
style nit: 4 space continuation indentation.
@rick-slin thanks, the changes look good to me. Left a small nit and a question. |
…ionOfSeconds added tests for TimeElementsWithFractionOfSeconds class
dfdatetime/precisions.py
Outdated
|
||
Returns: | ||
decimal.Decimal: fraction of second, which must be a value between 0.00 | ||
and 1.00. |
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.
style nit: missing continuation indentation
dfdatetime/precisions.py
Outdated
nanoseconds (int): number of nanoseconds. | ||
|
||
Returns: | ||
decimal.Decimal: fraction of second, which must be a value between 0.00 |
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.
no need for the addition 0 in 0.00
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.
LGTM
@joachimmetz As discussed in the plaso PR #4688, I would like to propose these change to add support for centisecond precision in TimeElementsWithFractionOfSeconds.
I think it would be easy to add 100us precision based on that template.
Thank you for your time and consideration.