Format length as M:SS by default #1749
Conversation
* Modify library.Item in order to have length formatted as H:MM instead of the raw number of seconds by using a types.Float subclass (DurationType). * Add library.DurationType, with custom format() and parse() methods that handle the conversion. * Add dbcore.query.DurationQuery as a NumericQuery subclass that _convert()s the ranges specified by the user to floats, delegating the rest of the functionality in the parent NumericQuery class. * Add ui.raw_seconds_short() as the reverse of human_seconds_short(). This function uses a regular expression in order to allow any number of minutes, and always required SS to have two digits.
* Add "format_raw_length" global configuration option to allow the user to toggle between human-readable (default) or raw formatting of track durations.
| Raises ValueError if the conversion cannot take place due to `string` not | ||
| being in the right format. | ||
| """ | ||
| match = re.match('^(\d+):([0-5]\d)$', string) |
There was a problem hiding this comment.
A note about using a regexp: I have decided to go through this route to make it consistent with human_seconds_short (ie. format should be (int)M:(int)SS, where SS must be < 60). Using the datetime facilities would restrict M to be < 60.
It might be desirable to make it a bit more flexible (allow HH:MM:SS, or M:SS.zzzzz, etc), and it should not be too hard, but I've opted not to include those for keeping it as an "inverse" of sorts of raw_seconds_short.
There was a problem hiding this comment.
Perfect. This seems good for now—and hour-long tracks are certainly an edge case.
|
Also, this change might reveal a small output discrepancy on the This seems to be due to printing the values from the |
| return None | ||
| try: | ||
| # TODO: tidy up circular import | ||
| from beets.ui import raw_seconds_short |
There was a problem hiding this comment.
Good point on the circular import—and we mostly try to make dbcore a separate component. Maybe the function should live in this module?
|
Fantastic! You beat me to it. 😃 This looks wonderful. For this feature, white-box unit tests seem like the right approach to me. You've even updated one case that looks more like an integration test, so I think that should be fine. And take your time! There's clearly no rush, so no need to apologize for real-life responsibilities. |
As for the circular import issue mentioned on #1749 (comment), I have moved The tests are not too extensive but should cover the basic |
|
This looks perfect, and the tests will be a great addition by themselves. Sorry for the delay, but I'm merging this now! |
Format length as M:SS by default
* master: (94 commits) Clean up changelog for edit plugin scrub: Demote a log message to debug scrub: Restore tags & art in auto mode (beetbox#1657) scrub: Run on import in auto mode (beetbox#1657) Fix beetbox#1673: Escape regex terms in lyrics snake_case variable names Doc refinements for beetbox#1749 Remove tests for Google fetchart backend (beetbox#1760) fetchart: Remove Google backend (fix beetbox#1760) fetchart: Better logging for iTunes (beetbox#1760) fetchart: Fix beetbox#1610: itunes install docs Fix test that depended on local time, 2 Fix test that depended on local time Fix unused import leftover on test_library Add documentation for M:SS length Move raw_seconds_short to beets.util Add tests for library-specific field types Fix test that was expecting raw length format Fix pyflakes issues, variable name Add config option for human vs raw track duration ...
As a followup to #1689 and #1737, here is a work-in-progress for trying to make the track lengths be displayed as
M:SSinstead of the raw number of seconds (and miliseconds).I'm assuming the intent is that the change is solely a visual one - database should still store the value as a float. To accomplish this, I have followed your suggestion and set
Item.lengthas a customDurationType(types.Float)withformat()andparse()methods that handle the conversion between H:MM and float formats (which has been added touito avoid some duplication). Additionally, I have added aDurationQuery(NumericQuery)in order to be able to perform queries using both formats, ie:I'd welcome some pointers about potential implications that I might have missed (as I'm still not sure if this change might break some functionality that depends on the length formatting), and ideas about integration tests or any other test to ensure everything works as intended. It's been a busy week on my end (and next one will probably be - sorry about that), but I plan to add at least white-box tests (basically, for
library.DurationType, though I'm tempted to also add them forDateType,PathTypeandMusicalKey, as they are missing) in the next few days.