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.53552% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.76%. Comparing base (dbe97c2) to head (9b1bd5d).
⚠️ Report is 14 commits behind head on master.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
beetsplug/discogs/__init__.py	80.85%	7 Missing and 2 partials ⚠️
beetsplug/discogs/states.py	98.97%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6179      +/-   ##
==========================================
+ Coverage   68.73%   68.76%   +0.03%     
==========================================
  Files         138      140       +2     
  Lines       18555    18610      +55     
  Branches     3058     3051       -7     
==========================================
+ Hits        12753    12797      +44     
- Misses       5152     5165      +13     
+ Partials      650      648       -2     

Files with missing lines	Coverage Δ
beetsplug/discogs/types.py	100.00% <100.00%> (ø)
beetsplug/discogs/states.py	98.97% <98.97%> (ø)
beetsplug/discogs/__init__.py	64.87% <80.85%> (ø)

... 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]

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.

[snejus]

Sorry that it took me a long time to finally get to this! Well done, this is a big and important piece of work.

It seems to me that DiscogsPlugin now has a bit too many responsibilities. I think it would be a good idea to move the logic from _build_* methods to intermediate dataclasses which should make it easier for us to reason about it all and hopefully simplify the logic. I refactored a couple of bits as I reviewed this, so I'm opening a PR based from your last commit for you to review!

[snejus]

See #6277

[henry-oberholtzer]

Split the plugin into four files since it's been feeling a bit unwieldy to sort through. Happy to undo or change the reorganization, but I think it's a bit easier to understand and navigate now.

[snejus]

Mostly looks good - we will want to move the subtracks merging functionality under TracklistState but this can be done in a separate PR :)

[snejus]

Just need to rebase now :)

[henry-oberholtzer]

Glad to finally have this fix in, now I can start thinking about all the other little improvements that are still left to make on the plugin...