Respect artist_credit when importing
#5916
Conversation
|
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❌ Patch coverage is 🚀 New features to boost your workflow:
|
mikeroll
force-pushed
the
artist-credit
branch
from
0728a1c to
4aeb57c
Compare
|
@mikeroll that is indeed a lot of duplication. I think a better approach would be to define an attribute on @property
def artist(self) -> str | None:
artist_credit = (
self["artist_credit"] if beets.config["artist_credit"] else None
)
return artist_credit or self["artist"]
@artist.setter
def artist(self, value: str) -> None:
self["artist"] = valueLet me first refactor these two classes to isolate common functionality into a common |
### Context See #5916 where we've come across a need to define common logic between `TrackInfo` and `AlbumInfo`. ### Changes - Introduce generic `Info` base (extends `AttrDict`) used by `AlbumInfo` / `TrackInfo` to centralize shared attributes and initialisation logic. - Sort keyword parameters in each constructor alphabetically and make them explicit. - Deduplicate and simplify shared `copy()` method using `copy.deepcopy` - Improve type hints and documentation. - Drop unused logging artifacts. ## Summary by Sourcery Refactor metadata-handling classes by extracting common functionality into a new Info base, updating AlbumInfo and TrackInfo to extend it with explicit sorted parameters, unify their copy logic, improve type annotations and docs, and drop obsolete logging code New Features: - Introduce a generic Info base class to centralize shared logic for AlbumInfo and TrackInfo Enhancements: - Alphabetically sort and explicitly declare constructor keyword parameters for consistency - Unify and simplify the copy() implementation in AttrDict using deepcopy - Enhance type hints and documentation for metadata classes Chores: - Remove unused logging imports and artifacts
|
@mikeroll forgot to mention that my PR has already been merged. Try to rebase on master try my suggestion above |
Description
Fixes #5010. Fixes #5633.
I've started with the most naive solution - this is essentially the same piece of logic in five places - as I'm not yet feeling comfortable to mess with
AlbumInfo/TrackInfo, nor have I found an optimal place to do so. Some pointers on centralizing this (if it makes sense?) are welcome!I would proceed adding the tests after the initial feedback. Thanks!
To Do
Documentation