beets icon indicating copy to clipboard operation
beets copied to clipboard

Added ftintitle_mb plugin

Open dvcky opened this issue 3 years ago • 14 comments
trafficstars

Added ftintitle_mb plugin

After conversing and thinking over my original idea of (overhauling ftintitle) with @sampsyo, I decided it would make more sense to have this be an entirely different plugin.

Description

This plugin moves "featured" artists to the title from the artist field. Unlike the previous ftintitle, ftintitle_mb uses MusicBrainz data to determine where the featured artist lies. It also uses new hooks that reduce redundancy in fetching MusicBrainz data.

Additional notes

Configuration variables:

  • feat_format: Determines how to format the feature at the end of the title. Default value: "(feat. {0})"
  • collab_cases: Allows certain "feature types" to be added to the artist before switching to the title. Default value: [", ", " & ", " vs ", " x ", " × "]

The new hooks added:

  • matchinfo_received: returns user-selected Match object (match)
  • mb_album_by_id_received: returns unmodified MusicBrainz album data (info)
  • mb_track_by_id_received: returns unmodified MusicBrainz track data (info)
  • mb_album_by_search_received: returns unmodified MusicBrainz search data for albums (info)
  • mb_track_by_search_received: returns unmodified MusicBrainz search data for tracks (info)

~~### Also: For some reason, I can't pass a custom collab_cases through my user config. I have the default value in the plugin set to something I believe is a reasonable default, but I do want it to be customizable to the user's preference. I thought bringing this to the pull request might draw some attention and get a fresh set of eyes to look at and give tips on the code. Any help would be greatly appreciated!~~ I can't type, that's the unfortunate reason for this issue. It's fixed now, and everything is working great as far as I can tell! Should be ready for merge!

dvcky avatar Oct 26 '22 22:10 dvcky

Would love to get approval for this workflow and some opinions on how to get collab_cases working correctly!

dvcky avatar Nov 19 '22 19:11 dvcky

Awesome; thanks for getting this started!

Odd thing about the configuration not being picked up. Nothing looks out of place at first glance… what seems to be the problem when you try it out? Is the default not working, or is your YAML configuration file not getting interpreted as you expect? If it's the latter, maybe you could share a snippet of YAML showing what you're doing?

sampsyo avatar Nov 19 '22 20:11 sampsyo

@sampsyo The configuration I set as the default seems to work just fine, and if I modify it the program reflects those changes correctly. Like you said, it has issues when I try to use a custom YAML configuration file to overwrite those changes. Edit: sorry I forgot to add that my user config is set like this:

ftinttitle_mb:
    collab_cases: [" foo ", "bar"]

The hope is that the program would switch from checking the default values ([", ", " & ", " vs ", " x ", " × "]) to my custom set ones ([" foo ", "bar"]). Unfortunately it does not.

dvcky avatar Nov 19 '22 20:11 dvcky

Any chance it's attributable to a spelling error? ftinttitle_mb has an extra t in it.

sampsyo avatar Nov 19 '22 21:11 sampsyo

Any chance it's attributable to a spelling error? ftinttitle_mb has an extra t in it.

Facepalms...yep, that was exactly what the issue was! It seems so obvious now, but I guess when you stare at something for too long you end up missing the most obvious details. Thanks for pointing it out! Ran a couple tests, and the plugin seems to be working great! As far as I can tell, this should be all set to merge!

dvcky avatar Nov 19 '22 21:11 dvcky

Any tips for the linting issues? I seem like it happens a bit where I getting errors on my computer that the linter for this repository doesn't, and vise-versa. So I'm kinda shooting in the dark a bit trying to fix these.

That being said, I would love a review and consideration for merge!

dvcky avatar Nov 20 '22 21:11 dvcky

That's odd! There is a thing about flake8 versions… hopefully you'll get the same results locally if you pip install -U flake8 to get the most recent version?

sampsyo avatar Nov 21 '22 17:11 sampsyo

I figured out what my issue is, I'm not getting flake8 "D errors" (presumably documentation errors). How do I get those to show up when running flake8? Hopefully this last push above solved everything though.

dvcky avatar Nov 21 '22 18:11 dvcky

~~just putting a heads up that I actually don't want to merge this plugin just yet - unfortunately, the MusicBrainz API was slightly modified in a way that changes how features are displayed in data. Because musicbrainzngs hasn't updated to support the new "joinphrase" attribute, my code will not work :( so I'll put a pin in this until that is updated. unfortunate, but I don't want to push broken code.~~ (read comment below)

dvcky avatar Dec 09 '22 04:12 dvcky

Turns out I just wrote the test class a bit wrong, whoops! This should be fully functional, and now includes the changelog addition as well as a testing class as shown in the commit earlier. I don't have any command flags (yet), so there is no documentation needed on that front. Is there anything else I should add to get this ready for merge @sampsyo?

dvcky avatar Dec 10 '22 16:12 dvcky

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar May 01 '23 22:05 stale[bot]

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Oct 15 '23 16:10 stale[bot]

Admittedly, I have not touched this in quite a long time. It would be cool if it was pushed, but at this point I have other projects on my mind so it is not a focus for me. Entirely the call of the main developers of this software though, and I can always try my best to provide context or an explanation on anything if needed!

dvcky avatar Oct 16 '23 18:10 dvcky

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Mar 17 '24 13:03 stale[bot]