Repository: beetbox/beets
Pull request: Make `%aunique` work with `-f/--format` options.
This should fix #3042. Let's see if Travis gives the thumbs up...

Problem statement:
%aunique fails when listing albums
### Problem

```
$ beet ls -a -f '$albumartist - $album%aunique{}'
Weezer - Make Believe<no such field 'album_id'>
Weezer - Maladroit<no such field 'album_id'>
Weezer - Pinkerton<no such field 'album_id'>
Weezer - Raditude<no such field 'album_id'>
Weezer - Weezer<no such field 'album_id'>
Weezer - Weezer<no such field 'album_id'>
Weezer - Weezer<no such field 'album_id'>
```

Looks like `aunique` assumes that it'll only ever handle `Item`s (albums don't have an `album_id` field).

This seems like a simple fix. I should have a pull request ready today or tomorrow.

Hints from issue discussion:
Sounds great! This seems like a nice, straightforward addition.

Commit message: Add a reference to the bug.

Patch to review:
diff --git a/beets/library.py b/beets/library.py
index d0571ed3a3..c911bbaa51 100644
--- a/beets/library.py
+++ b/beets/library.py
@@ -1395,6 +1395,8 @@ def get_album(self, item_or_id):
         """
         if isinstance(item_or_id, int):
             album_id = item_or_id
+        elif isinstance(item_or_id, Album):
+            album_id = item_or_id.id
         else:
             album_id = item_or_id.album_id
         if album_id is None:
@@ -1506,9 +1508,16 @@ def tmpl_aunique(self, keys=None, disam=None, bracket=None):
         # Fast paths: no album, no item or library, or memoized value.
         if not self.item or not self.lib:
             return u''
-        if self.item.album_id is None:
+
+        if isinstance(self.item, Item):
+            album_id = self.item.album_id
+        elif isinstance(self.item, Album):
+            album_id = self.item.id
+
+        if album_id is None:
             return u''
-        memokey = ('aunique', keys, disam, self.item.album_id)
+
+        memokey = ('aunique', keys, disam, album_id)
         memoval = self.lib._memotable.get(memokey)
         if memoval is not None:
             return memoval
diff --git a/docs/changelog.rst b/docs/changelog.rst
index 2d456ae886..4cf4072fe4 100644
--- a/docs/changelog.rst
+++ b/docs/changelog.rst
@@ -88,6 +88,9 @@ Fixes:
   :bug:`2504` :bug:`3029`
 * Avoid a crash when archive extraction fails during import.
   :bug:`3041`
+* The ``%aunique`` template function now works correctly with the
+  ``-f/--format`` option.
+  :bug:`3043`
 
 .. _python-itunes: https://github.com/ocelma/python-itunes
 


Code review comments:
diff_hunk: @@ -1395,6 +1395,8 @@ def get_album(self, item_or_id):
         """
         if isinstance(item_or_id, int):
             album_id = item_or_id
+        elif isinstance(item_or_id, Album):
+            album_id = item_or_id.id
path: beets/library.py
text: @user1:
If this function can now accept Album objects, maybe we should update the docstring to reflect that? (Also, I don’t quite see where the call is that makes this change necessary—maybe that call should be rewritten to just use the Album object it already has instead of going to the database to get another identical copy?)

@author:
The call to `lib.get_album()` comes from [tmpl_aunique](https://github.com/beetbox/beets/blob/3b78d933cf46d5cad8f90ea6cdf38ada9e10846f/beets/library.py#L1531) where `self.item` can be either an Item or an Album.

I couldn't think of a clean way of refactoring that method. I suppose `lib.get_album()`, if given an Album, could just return the same object. That would prevent hitting the database twice.

@user1:
How about using `lib.get_album(album_id)`? In your improved version of `tmpl_aunique`, we now have an `album_id` variable already. That won't prevent hitting the database twice, but it will avoid expanding the responsibilities of `get_album`.

@author:
Good idea! Pushed a new commit.