beets icon indicating copy to clipboard operation
beets copied to clipboard

An opinionated fix for "the albumtype(s) problem"

Open JOJ0 opened this issue 1 year ago • 5 comments

Description

Addresses parts of #4715

This fix is of the type "works for me" and I'd like to open the discussion and test for others. In my opinion most of the problems we have with albumtypes and albumtype fighting against each other is because it just does not make sense to save both of them in beets. No matter where the real problem is, in the end it should be sufficient to save the data once in the beets library, hence only in the multifield albumtypes. I'm proposing to kick out albumtype from beets entirely!

This fix does not require any changes to mediafile

Furthermore this fixes saving albumtypes for the Discogs, the Spotify and the Deezer plugin. Previously populating albumtype was implemented theoratically but broken, and albumtypes was not implemented. (I only recently realized this while working on this fix, please tell me if there is open issue for that already, so I can attach them to this PR, thanks!)

A nice side effect of the latter fix is that files that where tagged using one of those plugins, can finally be used correctly by the albumtypes plugin as well :tada:

To Do

  • [ ] Documentation.
  • [ ] Changelog
  • [ ] Fix ambiguation field to use albumtypes instead of albumtype
  • [ ] Fix existing Tests accordingly.

JOJ0 avatar Jan 10 '24 06:01 JOJ0

This albumtype issue is really annoying, and I am perfectly fine with this approach. One suggestion - to avoid breaking changes to other plugins, can we set albumtypes = albumtype if the plugin only updates albumtype.

arsaboo avatar Mar 14 '24 14:03 arsaboo

Not a contributor, nor is my knowledge of beets internals on a sufficient level to offer technical insight, but as someone who's been using this project for several years (thanks!), this fix seems fine. I don't care for the albumtype field.

randoragon avatar Mar 17 '24 19:03 randoragon

This albumtype issue is really annoying, and I am perfectly fine with this approach. One suggestion - to avoid breaking changes to other plugins, can we set albumtypes = albumtype if the plugin only updates albumtype.

I'm not sure if it's that easy. Will assigning the string from albumtype to the "multi-field" albumtypes work? You could investigate that in the initial PR that made albumtypes a proper multi-field: https://github.com/beetbox/beets/pull/4582

JOJ0 avatar Mar 18 '24 06:03 JOJ0

Not a contributor, nor is my knowledge of beets internals on a sufficient level to offer technical insight, but as someone who's been using this project for several years (thanks!), this fix seems fine. I don't care for the albumtype field.

Initially I thought I don't care about it either, but now I learned that we can't simply get rid of it, there is too many plugins that still want to use it! So, yeah basically what @arsaboo said is a good idea, let's discuss the technical ways to do that.

JOJ0 avatar Mar 18 '24 06:03 JOJ0

Just thinking aloud here.

How about having some helper function to translate between albumtype and albumtypes? If any plugin sets albumtype, we use that information to set the albumtypes and vice versa. In the db, we only store albumtypes.

arsaboo avatar Mar 18 '24 15:03 arsaboo