Refactored beets/random.py and moved into beetsplug/random.py

[semohr]

The beets/random.py module was only used by the random plugin, so I moved its functions into beetsplug/random.py to keep core modules cleaner.

Changes:

• Moved beets/random.py functions into beetsplug/random.py
• Added typehints for better readability and tooling support
• Added additional tests for improved coverage
• General tidy up and refactor, keeping the core functionality unchanged

[Copilot]

Pull Request Overview

This PR moves the beets/random.py module into beetsplug/random.py to consolidate random functionality with its only consumer, the random plugin, thereby keeping core modules cleaner.

• Moved all functions from beets/random.py into beetsplug/random.py
• Added comprehensive type hints for better code clarity and tooling support
• Enhanced test coverage with additional test cases for edge cases and different functionality modes

Reviewed Changes

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

File	Description
beets/random.py	Complete removal of the standalone random module
beetsplug/random.py	Integration of moved functions with added type hints and improved implementation
test/plugins/test_random.py	Updated import path and added comprehensive test cases for better coverage

[codecov]

Codecov Report

❌ Patch coverage is 93.61702% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.76%. Comparing base (1a899cc) to head (73655ca).
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
beetsplug/random.py	95.34%	2 Missing ⚠️
beets/library/models.py	75.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5924      +/-   ##
==========================================
+ Coverage   68.68%   68.76%   +0.07%     
==========================================
  Files         138      138              
  Lines       18532    18572      +40     
  Branches     3061     3067       +6     
==========================================
+ Hits        12729    12771      +42     
+ Misses       5149     5147       -2     
  Partials      654      654              

Files with missing lines	Coverage Δ
beets/library/models.py	87.08% <75.00%> (-0.08%)	⬇️
beetsplug/random.py	92.72% <95.34%> (ø)

🚀 New features to boost your workflow:

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

[snejus]

A couple of more comments. I found that unused default arguments are still present, so I unresolved my previous two comments where I commented about them.

[snejus]

Since you've added field argument, I added a couple of related comments. This should be mentioned in the changelog as well.

[snejus]

I gave this a test and found two bugs:

1. List fields are not handled

$ beet random --field artists -e
Traceback (most recent call last):
  File "/home/sarunas/.local/bin/beet", line 8, in <module>
    main()
  File "/home/sarunas/repo/beets/beets/ui/__init__.py", line 1629, in main
    _raw_main(args)
  File "/home/sarunas/repo/beets/beets/ui/__init__.py", line 1608, in _raw_main
    subcommand.func(lib, suboptions, subargs)
  File "/home/sarunas/repo/beets/beetsplug/random.py", line 39, in random_func
    for obj in random_objs(
  File "/home/sarunas/repo/beets/beetsplug/random.py", line 112, in _equal_chance_permutation
    groups[k] = list(values)
TypeError: unhashable type: 'list'```

2. items aren't sorted before grouping in equal chance permutation. See beet list output:

$ beet list path::aaa title- -f '$album: $artist - $title'
BLAH!!!! malakaa!! todotiernos de mierdaaa: DER SANDMANN - Triple Masala
BLAH!!!! malakaa!! todotiernos de mierdaaa: DER SANDMANN - Machete
BLAH!!!! malakaa!! todotiernos de mierdaaa: DER SANDMANN - Lost in Time
BLAH!!!! malakaa!! todotiernos de mierdaaa: DER SANDMANN - Freaks at Work
BLAH!!!! malakaa!! todotiernos de mierdaaa: DER SANDMANN - der killer wahl 185bpm
RAVE SLUTZ: Fallen Shrine & dj Christian NXC - DEEEJAAAY
BLAH!!!! malakaa!! todotiernos de mierdaaa: DER SANDMANN - Blah
Netikras albumas: Karpiz - AAA

vs

$ beet random path::aaa title- -f '$album: $artist - $title' --field album -e -n10
RAVE SLUTZ: Fallen Shrine & dj Christian NXC - DEEEJAAAY
Netikras albumas: Karpiz - AAA
BLAH!!!! malakaa!! todotiernos de mierdaaa: DER SANDMANN - Blah

I expected to see the same items but in random order.

I'm keen to finally wrap this up, so feel free to just paste this fix:

diff --git a/beetsplug/random.py b/beetsplug/random.py
index 9714c8e53..aa65dd5d5 100644
--- a/beetsplug/random.py
+++ b/beetsplug/random.py
@@ -19,4 +19,4 @@
 from itertools import groupby, islice
-from operator import attrgetter
-from typing import TYPE_CHECKING, Any
+from operator import methodcaller
+from typing import TYPE_CHECKING
 
@@ -86,5 +86,2 @@ def commands
 
-NOT_FOUND_SENTINEL = object()
-
-
 def _equal_chance_permutation(
@@ -97,22 +94,12 @@ def _equal_chance_permutation
     # Group the objects by field so we can sample from them.
-    key = attrgetter(field)
-
-    def get_attr(obj: LibModel) -> Any:
-        try:
-            return key(obj)
-        except AttributeError:
-            return NOT_FOUND_SENTINEL
-
-    sorted(objs, key=get_attr)
-
-    groups: dict[str | object, list[LibModel]] = {
-        NOT_FOUND_SENTINEL: [],
-    }
-    for k, values in groupby(objs, key=get_attr):
-        groups[k] = list(values)
-        # shuffle in category
-        random.shuffle(groups[k])
-
-    # Remove items without the field value.
-    del groups[NOT_FOUND_SENTINEL]
+    get_attr = methodcaller("get", field)
+
+    groups = {}
+    for k, values in groupby(sorted(objs, key=get_attr), key=get_attr):
+        if k is not None:
+            vals = list(values)
+            # shuffle in category
+            random.shuffle(vals)
+            groups[str(k)] = vals
+
     while groups:

[snejus]

LibModel is a dictionary - so this can be simplified with using LibModel.get instead of try/except + sentinel.

Suggested change

	key = attrgetter(field)
	def get_attr(obj: LibModel) -> Any:
	try:
	return key(obj)
	except AttributeError:
	return NOT_FOUND_SENTINEL
	sorted(objs, key=get_attr)
	groups: dict[str | object, list[LibModel]] = {
	NOT_FOUND_SENTINEL: [],
	}
	# Group the objects by field so we can sample from them.
	get_attr = methodcaller("get", field)
	sorted(objs, key=get_attr)
	groups = {}

[snejus]

This has no effect - you want to either move it to

    for k, values in groupby(sorted(objs, key=get_attr), key=get_attr):

or better: revert to what we had previously by casting entities to list and send list[LibModel] through:

diff --git a/beetsplug/random.py b/beetsplug/random.py
index 9714c8e53..44adc9fbe 100644
--- a/beetsplug/random.py
+++ b/beetsplug/random.py
@@ -33,7 +33,7 @@
 def random_func(lib: Library, opts: optparse.Values, args: list[str]):
     """Select some random items or albums and print the results."""
     # Fetch all the objects matching the query into a list.
-    objs = lib.albums(args) if opts.album else lib.items(args)
+    objs = list(lib.albums(args) if opts.album else lib.items(args))
 
     # Print a random subset.
     for obj in random_objs(

which allows

Suggested change

	sorted(objs, key=get_attr)
	objs.sort(key=get_attr)

[snejus]

1. Need to handle list fields like artists, albumtypes - we can just cast the keys to str
2. Skip missing (None) values removing the need to delete them later

Suggested change

	groups[k] = list(values)
	# shuffle in category
	random.shuffle(groups[k])
	# Remove items without the field value.
	del groups[NOT_FOUND_SENTINEL]
	if k is not None:
	vals = list(values)
	# shuffle in category
	random.shuffle(vals)
	groups[str(k)] = vals