Discogs: fixes for: 6177, 6068

[henry-oberholtzer]

Description

Fixes #6177, #6068.

I fixed the issue in #6177 by removing the derived class interface, and moving those fields back into function variables. They're a bit unwieldy still, but that's the algorithm it came with. There's a lot of room to continue to improve the clarity of the code in that section, but I think that'll require a deeper overhaul.

For #6068, I created the ArtistInfo and AlbumArtistInfo typed dictionaries, and was able to centralize the logic of building the artist info into build_artistinfo and build_albumartistinfo. Tests for these scenarios were created largely by expanding existing tests to incorporate the new fields.

I think I'll have to re-think the entire algorithm for 6030 to make it more flexible at parsing the issue for #6030, so I'll move that to a later PR in the interest of getting the flex attr fix in.

To Do

• Changelog. (Add an entry to docs/changelog.rst to the bottom of one of the lists near the top of the document.)
• Tests.

[codecov]

Codecov Report

❌ Patch coverage is 94.26752% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.70%. Comparing base (1a899cc) to head (4ac4e19).
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
beetsplug/discogs.py	94.26%	7 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6179      +/-   ##
==========================================
+ Coverage   68.68%   68.70%   +0.01%     
==========================================
  Files         138      138              
  Lines       18532    18600      +68     
  Branches     3061     3069       +8     
==========================================
+ Hits        12729    12779      +50     
- Misses       5149     5169      +20     
+ Partials      654      652       -2     

Files with missing lines	Coverage Δ
beetsplug/discogs.py	74.40% <94.26%> (+3.62%)	⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[semohr]

I just had some time to have a look here. Added some comments for potential improvements.

Thanks for starting to tackle the discogs plugin, it is in dire need of some love 😄

[henry-oberholtzer]

Thanks Sebastian! I'm out of town at the moment but will get these applied once I'm back.

[semohr]

No hurry! Take your time and enjoy the holidays.

[semohr]

Can we try to not use single character variable names? I get that they simplify things in writing but my fish brain forgets what t is after reading like 20 lines 🤣

[henry-oberholtzer]

Agreed - they're fine for hashing out the ideas but they do make retreading the code a lot harder.

[semohr]

I think we should good here!

While the whole logic is still a bit convoluted, I feel like it is a major improvement from before the PR 👍

Thank you for taking the initiative!

[henry-oberholtzer]

Updated the variable name - can go ahead and merge if all looks good!

[semohr]

I think @snejus wants to have a look too since he self assigned this to himself too.