Introduce Info.name property and add types to match details functions
Generalize some of common tagging functionality to Info and Match base classes
This PR centralises some common tagging functionality between singletons and albums allowing to simplify ChangeRepresentation and importing functionality. This is prep work for a larger PR which refactors and simplifies the entire tagging workflow.
Changes
-
Core type changes: Changed
mappingparameter fromMapping[Item, TrackInfo]tolist[tuple[Item, TrackInfo]]inapply_metadata(),distance(), andassign_items()functions -
Match dataclasses: Converted
AlbumMatchandTrackMatchfromNamedTupleto@dataclass, introducing baseMatchclass with common functionality -
New properties: Added
namecached property toInfoclass for unified name access -
ChangeRepresentation refactor: Converted to
@dataclasswith lazy property evaluation, replacingcur_album/cur_titlewith unifiedcur_namefield -
UI improvements: Simplified display logic by using
match.info.nameinstead of type-specific field checks -
Parameter renaming: Renamed
search_album/search_titleparameters tosearch_namefor consistency across singleton and album workflows
The changes maintain backward compatibility in behavior while improving type safety and code clarity.
Thank you for the PR! The changelog has not been updated, so here is a friendly reminder to check if you need to add an entry.
Codecov Report
:x: Patch coverage is 95.79832% with 5 lines in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 68.26%. Comparing base (b058218) to head (60b4a38).
:warning: Report is 6 commits behind head on master.
:white_check_mark: All tests successful. No failed tests found.
| Files with missing lines | Patch % | Lines |
|---|---|---|
| beetsplug/bpsync.py | 0.00% | 2 Missing :warning: |
| beetsplug/mbsync.py | 60.00% | 2 Missing :warning: |
| beets/autotag/match.py | 87.50% | 0 Missing and 1 partial :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## master #6142 +/- ##
==========================================
+ Coverage 68.22% 68.26% +0.03%
==========================================
Files 138 138
Lines 18772 18791 +19
Branches 3170 3167 -3
==========================================
+ Hits 12808 12827 +19
Misses 5290 5290
Partials 674 674
| Files with missing lines | Coverage Δ | |
|---|---|---|
| beets/autotag/__init__.py | 87.61% <100.00%> (ø) |
|
| beets/autotag/distance.py | 82.59% <100.00%> (ø) |
|
| beets/autotag/hooks.py | 100.00% <100.00%> (ø) |
|
| beets/importer/tasks.py | 90.89% <100.00%> (ø) |
|
| beets/ui/commands/import_/display.py | 81.85% <100.00%> (-0.27%) |
:arrow_down: |
| beets/ui/commands/import_/session.py | 58.62% <100.00%> (ø) |
|
| beetsplug/mbpseudo.py | 79.87% <100.00%> (-0.13%) |
:arrow_down: |
| beets/autotag/match.py | 76.92% <87.50%> (ø) |
|
| beetsplug/bpsync.py | 18.07% <0.00%> (ø) |
|
| beetsplug/mbsync.py | 81.81% <60.00%> (ø) |
:rocket: New features to boost your workflow:
- :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
Just had a short look and this will absolutly break beets-flask downstream. We use the mappings dict quite a bit... Can we move this change in particular to a different PR?
I think the other things are mainly no brainers.
I need to think abit about the implications a how to migrate this in beets-flask. Just a bit more time here would be highly appreciated :)
Just had a short look and this will absolutly break beets-flask downstream. We use the mappings dict quite a bit... Can we move this change in particular to a different PR?
I think the other things are mainly no brainers.
I need to think abit about the implications a how to migrate this in beets-flask. Just a bit more time here would be highly appreciated :)
No worries - I kept Match.mapping the same now - instead, I introduced item_info_pairs property.
Renaming does not really help me here :laughing:
Where is the usecase for list[tuple[Item, TrackInfo]] over Mapping[Item, TrackInfo]? I feel like the later is cleaner if there is not a specific edgecase I'm missing.
Renaming does not really help me here 😆
I'm not sure that I understand - AlbumMatch.mapping is now present like it was before. I just introduce a new property item_info_pairs:
@dataclass
class AlbumMatch(Match):
info: AlbumInfo
mapping: dict[Item, TrackInfo]
extra_items: list[Item]
extra_tracks: list[TrackInfo]
@property
def item_info_pairs(self) -> list[tuple[Item, TrackInfo]]:
return list(self.mapping.items())
Where is the usecase for
list[tuple[Item, TrackInfo]]overMapping[Item, TrackInfo]? I feel like the later is cleaner if there is not a specific edgecase I'm missing.
It's just a wrong data type for how this property is used across the codebase - it's used for recording and looping across matching (item, track_info) pairs for the album. No functionality in the codebase makes use of the mapping nature of it - instead, values are accessed as mapping.items() which just gets the underlying list of (item, track_info) pairs.
I thus clarified it with the new property.
Is there anything missing here @semohr?
Btw I really appreciate that you started to properly type some parts in the commands submodule. I was hopeing someone would start with this after the bigger refactor :P