beets
beets copied to clipboard
beetbox
LibModel.items() confusion
The docs describe LibModel.items() to
Iterate over (key, value) pairs that this object contains. Computed fields are not included.
This is the case for Item.items(), but Album.items() gives all the Items associated with the Album.
LibModel.items() is actually defined in Model.items(), and overwritten in the Album class.
So I'm wondering what the intended behavior is :)
I know I have used Album.items() as it currently is in the past (and it seems to me a sensibly named function). Just now I tried to use it in the sense defined in the docs and ran into this problem (although I want computed fields so not using it any more). In the docs sense I guess the items() mimics dict.items(), but does create some confusion with Items :D
You've hit the nail on the head. It's an unfortunate naming collision—one that exists mainly for historical reasons. Namely, the Item and Album objects predate dbcore; we refactored the latter out of the core beets library to bring a little more abstraction and modularity. So the Album.items() method is very old and special-purpose, while the new(er) Model.items() is generic.
So yes, it's not a great situation, but it's not clear what we should do instead. Perhaps rename Album.items() to something else someday?
So yes, it's not a great situation, but it's not clear what we should do instead. Perhaps rename
Album.items()to something else someday?
While unfortunate; this seems like a change that would break many third-party plugins. In my opinion, we should avoid renaming .items() (unless there's a stronger reason than being somewhat confusing). Maybe extending the Album.items() docstring to point this out would help to alleviate the issue?
Yep I think just documenting the conflict would be good. Doesn't seem like it's been too much of an issue in the past.
Yep I think just documenting the conflict would be good. Doesn't seem like it's been too much of an issue in the past.
Would you be able to make a quick PR for this?