Skip to content

Slightly simplify lastgenre client#6495

Open
snejus wants to merge 4 commits intomasterfrom
lastgenre-use-a-single-fetch-method
Open

Slightly simplify lastgenre client#6495
snejus wants to merge 4 commits intomasterfrom
lastgenre-use-a-single-fetch-method

Conversation

@snejus
Copy link
Copy Markdown
Member

@snejus snejus commented Apr 4, 2026

Consolidate Last.fm genre fetching into a single fetch method

This PR simplifies the lastgenre client API by replacing three separate fetch methods (fetch_track_genre, fetch_album_genre, fetch_artist_genre) with a single unified fetch(kind, obj) method.

What changed

client.py:

  • Introduces a class-level FETCH_METHODS registry (ClassVar dict) mapping fetch "kinds" ("track", "album", "artist", "album_artist") to a (pylast_method, arg_extractor) tuple.
  • Replaces the three fetch_* methods with a single fetch(kind, obj) that dispatches via this registry.
  • Removes a private _tags_for wrapper — its logic is inlined into the now-public fetch_genres.
  • Drops the workaround for a pylast.Album.get_top_tags() inconsistency fixed in 2014.

__init__.py:

  • All call sites updated to use client.fetch(kind, obj) — the client now owns field extraction (e.g. obj.artist, obj.album), removing that concern from the plugin layer.

test_lastgenre.py:

  • Test mocking simplified: a single monkeypatch on LastFmClient.fetch replaces three separate method patches.

Impact

  • Reduced surface area: one method to mock, test, and reason about instead of three.
  • Field extraction centralised: callers no longer need to know which fields to pass per entity type.
  • No behaviour change — pure refactor.

Copilot AI review requested due to automatic review settings April 4, 2026 00:22
@snejus snejus requested a review from a team as a code owner April 4, 2026 00:22
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 4, 2026

Thank you for the PR! The changelog has not been updated, so here is a friendly reminder to check if you need to add an entry.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

PR make lastgenre Last.fm client API smaller. Instead of 3 fetch funcs, now one fetch(kind, obj) dispatch table, and plugin call sites updated to use it.

Changes:

  • Add LastFmClient.FETCH_METHODS registry and new fetch(kind, obj) dispatcher.
  • Inline old tag parsing into fetch_genres and remove old per-entity fetch methods.
  • Update plugin + tests to use new fetch API and simpler mocking.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
beetsplug/lastgenre/client.py Add unified fetch dispatch + simplify tag fetch/filter path
beetsplug/lastgenre/init.py Switch genre resolution stages to call client.fetch(kind, obj)
test/plugins/test_lastgenre.py Update tests to new API + monkeypatch LastFmClient.fetch

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 4, 2026

Codecov Report

❌ Patch coverage is 58.82353% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.40%. Comparing base (fd586ef) to head (263ae8f).
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
beetsplug/lastgenre/client.py 60.00% 3 Missing and 1 partial ⚠️
beetsplug/lastgenre/__init__.py 57.14% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #6495   +/-   ##
=======================================
  Coverage   70.40%   70.40%           
=======================================
  Files         148      148           
  Lines       18806    18798    -8     
  Branches     3067     3066    -1     
=======================================
- Hits        13240    13235    -5     
+ Misses       4916     4913    -3     
  Partials      650      650           
Files with missing lines Coverage Δ
beetsplug/lastgenre/__init__.py 81.67% <57.14%> (ø)
beetsplug/lastgenre/client.py 53.65% <60.00%> (-1.45%) ⬇️
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@JOJ0 JOJ0 added the plugin Pull requests that are plugins related label Apr 4, 2026
@snejus snejus added lastgenre lastgenre plugin and removed plugin Pull requests that are plugins related labels Apr 4, 2026
@snejus snejus force-pushed the lastgenre-use-a-single-fetch-method branch 2 times, most recently from a2e3036 to 570bb75 Compare April 7, 2026 16:27
@snejus snejus requested a review from JOJ0 April 7, 2026 16:28
@snejus
Copy link
Copy Markdown
Member Author

snejus commented Apr 7, 2026

@JOJ0 I also added you as the lastgenre codeowner :)

@JOJ0
Copy link
Copy Markdown
Member

JOJ0 commented Apr 9, 2026

@snejus please rebase on master and let's get this in. That's the simplification you mentioned a while ago! Great! Thanks!

@snejus snejus force-pushed the lastgenre-use-a-single-fetch-method branch from ece5ae5 to 1fd4d03 Compare April 9, 2026 23:56
snejus added 4 commits April 10, 2026 09:21
Delegate the responsibility of getting relevant model fields to the
client by declaring the genre fetching spec on the class.
This issue has been resolved in 2014.
@snejus snejus force-pushed the lastgenre-use-a-single-fetch-method branch from 1fd4d03 to 263ae8f Compare April 10, 2026 08:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lastgenre lastgenre plugin

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants