port/MycroftAI/lingua-franca/pull/232 #27
Conversation
Improve nice_relative_time tests - Add more variations - Remove import from mycroft-core - Remove reference to local time to ensure deterministic outcome Ensure relative datetimes are compared in the same timezone. All comparisons are made in UTC to prevent issues of the two datetimes passed in having different timezone info. Remove unnecessary duplicate function The nice_relative_time function is already defined in the top level format.py hence it isn't needed in format_en.py. This convention may change in the future. Added general nice_relative_time_en Added test for function nice_relative_time Added function nice_relative_time (used in skill-alarm but not localized)
Codecov Report
@@ Coverage Diff @@
## dev #27 +/- ##
=====================================
Coverage ? 0.00%
=====================================
Files ? 65
Lines ? 16367
Branches ? 0
=====================================
Hits ? 0
Misses ? 16367
Partials ? 0 Continue to review full report at Codecov.
|
There was a problem hiding this comment.
I'm not sure I agree with always rounding to one unit; if I request something in "2 hours and 15 minutes" replying with "in 2 hours" would indicate to me that the "and 15 minutes" was mis-transcribed..
Maybe this could optionally accept a "precision" param? Caller could specify 'hours'/'minutes'/'seconds', etc.
Unported work from mycroft.util.lang introduced the relevant enum, but I don't think anyone ever used it |
Yeah, I think the current implementation is the simplest format that might be useful, but it still depends on the specific use case. Definitely lots of room for improvement here. I'm also interested to look at how this relates to |
Didn't think to look, but I do see that now..
Maybe it would make sense to deal with total seconds and some default format in both cases? Something like: def _get_spoken_duration(total_seconds: int, precision: TimeResolution):
# Do formatting here
def nice_relative_time(when: datetime, relative_to: datetime):
if relative_to:
now = relative_to
else:
now = now_local()
delta = to_local(when) - now
if delta.total_seconds() < 1:
return "now"
if delta.total_seconds() < 90:
if delta.total_seconds() == 1:
return "one second"
else:
return _get_spoken_duration(delta.total_seconds(), TimeResolution.SECONDS
...
def nice_duration(duration, lang='', speech=True, precision: TimeResolution=TimeResolution.SECONDS):
days = int(duration // 86400)
hours = int(duration // 3600 % 24)
minutes = int(duration // 60 % 60)
seconds = int(duration % 60)
if speech:
out = ""
if days > 0 and precision>=TimeResolution.DAYS:
... |
|
@coderabbitai review |
Actions performedReview triggered.
|
|
Warning Rate limit exceeded@JarbasAl has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 10 minutes and 33 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughA new function Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- lingua_franca/format.py (2 hunks)
- lingua_franca/lang/format_eu.py (1 hunks)
- test/unittests/test_format_en.py (2 hunks)
Additional context used
Ruff
lingua_franca/format.py
601-604: Use ternary operator
now = relative_to if relative_to else now_local()instead ofif-else-blockReplace
if-else-block withnow = relative_to if relative_to else now_local()(SIM108)
Additional comments not posted (2)
lingua_franca/lang/format_eu.py (1)
336-337: Clarification added tonice_relative_time_eufunction is clear and informative.The note explaining the limitation of the
nice_relative_time_eufunction to days is crucial for setting the correct expectations for the function's users. This is a good addition for clarity.test/unittests/test_format_en.py (1)
538-571: Comprehensive tests fornice_relative_timefunction.The tests cover various scenarios, ensuring that the function behaves as expected across different time differentials. This is a well-implemented test suite that should effectively catch regressions or errors in the
nice_relative_timefunction.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
port MycroftAI#232
(note, basque was already localized in #3 )
Summary by CodeRabbit
New Features
nice_relative_timefunction for generating human-readable time differences in the app.Documentation
nice_relative_time_eufunction to specify time reporting limitations.Tests
nice_relative_timeto validate various time differentials.