Support multiple units in format_timedelta()#1066
Support multiple units in format_timedelta()#1066hartmutobendorf wants to merge 2 commits intopython-babel:masterfrom
Conversation
…d (depth=full for all non-zero units, depth=fullest for all)
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1066 +/- ##
==========================================
- Coverage 90.99% 90.94% -0.05%
==========================================
Files 26 26
Lines 4444 4453 +9
==========================================
+ Hits 4044 4050 +6
- Misses 400 403 +3 ☔ View full report in Codecov by Sentry. |
akx
left a comment
akx
left a comment
There was a problem hiding this comment.
Thanks!
The depth names don't sound great to me, and this would really require some tests, in various locales too.
| threshold: float = .85, | ||
| add_direction: bool = False, | ||
| format: Literal['narrow', 'short', 'medium', 'long'] = 'long', | ||
| depth: Literal['shallow', 'full', 'fullest'] = 'shallow', |
There was a problem hiding this comment.
I'm not sure these names are the best or most descriptive. No matter what they end up being, they do need to be documented, though.
| return pattern.replace('{0}', str(value)) | ||
|
|
||
| return '' | ||
| if (depth=='shallow'): |
There was a problem hiding this comment.
| if (depth=='shallow'): | |
| if depth == 'shallow': |
| if (depth=='shallow'): | ||
| formatted_string = ' '.join(filter(None, [formatted_string, pattern.replace('{0}', str(value))])) | ||
| break | ||
| elif ((depth=='full' and value > 0) or depth == 'fullest'): |
There was a problem hiding this comment.
| elif ((depth=='full' and value > 0) or depth == 'fullest'): | |
| elif (depth == 'full' and value > 0) or depth == 'fullest': |
|
|
||
| return '' | ||
| if (depth=='shallow'): | ||
| formatted_string = ' '.join(filter(None, [formatted_string, pattern.replace('{0}', str(value))])) |
There was a problem hiding this comment.
I'm not sure I understand what's going on in this magic incantation 😄
It's repeated two lines underneath this, so maybe it should be DRYed out too?
2 participants
Issue #728 describes returning measures for several units when using format_timedelta().
E.g. instead of
return for depth=="fullest"
3 hours 0 minutes 40 secondsand for depth=="full"
3 hours 40 secondsTests are passing, but I haven't tried contributing to babel before, and am not aware of any additional requirements beyond the few lines of source code in this PR. Feedback appreciated.