Background
The `_legalize_stage` function was causing issues with Mypy due to
inconsistent type usage between the `path` and `extension` parameters.
This inconsistency stemmed from the `fragment` parameter influencing the
types of these variables.
Key issues
1. `path` was defined as `str`, while `extension` was `bytes`.
2. Depending on `fragment`, `extension` could be either `str` or `bytes`.
3. `path` was sometimes converted to `bytes` within `_legalize_stage`.
Item.destination` method
- The `fragment` parameter determined the output format:
- `False`: Returned absolute path as bytes (default)
- `True`: Returned path relative to library directory as str
Thus
- Rename `fragment` parameter to `relative_to_libdir` for clarity
- Ensure `Item.destination` returns `bytes` in all cases
- Code expecting strings now converts the output to `str`
- Use only `str` type in `_legalize_stage` and `_legalize_path`
functions
- These functions are no longer dependent on `relative_to_libdir`
## Description
This might be a quick one, depending on how you feel about it... It
allows you to pickle DB model objects. I don't think this is used
directly in Beets, but it might be useful in general. For instance, we
encountered an issue where we wanted to quickly pickle an Item or Album.
This sometimes worked and other times failed, which seemed quite
inconsistent.
Some DB model methods and properties have the side effect of attaching
an SQLite connection to self (._db), which prevents serialization. The
fix is quite straightforward, so I thought we might want to integrate
this into beets directly.
## To Do
- [x] Changelog
- [x] Tests
Using the correct function signature for g_file_new_for_path fixes the
tests on s390x.
I do not have the full story on why this failed consistently only on
s390x, but I guess the big endian might have something to play with
this.
Here is how the tests were failing:
```
169s ___________________________ ThumbnailsTest.test_uri ____________________________
169s
169s self = <test.plugins.test_thumbnails.ThumbnailsTest testMethod=test_uri>
169s
169s def test_uri(self):
169s gio = GioURI()
169s if not gio.available:
169s self.skipTest("GIO library not found")
169s
169s > assert gio.uri("/foo") == "file:///" # silent fail
169s E AssertionError: assert '' == 'file:///'
169s E
169s E - file:///
169s
169s test/plugins/test_thumbnails.py:268: AssertionError
```
You can see a full log here [1] and a history of consistent failure
here [2]. Both links are bound to expire at some point, sorry future
archeologist 🤷.
[1]: https://autopkgtest.ubuntu.com/results/autopkgtest-plucky/plucky/s390x/b/beets/20250403_162414_5d1da@/log.gz#S5
[2]: https://autopkgtest.ubuntu.com/packages/beets/plucky/s390x
This was not thought through clearly before. It now behaves as follows
which I suppose is least surprising to a user:
- force is on, keep_existing is on, but the whitelist is DISABLED
- no stage found anything on last.fm
- fall back to the original genre
If in this example the whitelist would be ENABLED, the behaviour
changes: Only if the existing genre passes the whitelist test the
original is kept.
If no album was found the next stage (artist) should be entered, the
original genre kicked out (not whitelisted) and artist genre accepted.
The log message is slightly misleading since it tried to keep existing
genres but they were not whitelisted, thus kicked out. This is expected.
- Rename method from _combine_genres() to _combine_resolve_and_log() to
make clear that it not only combines new and old genres but also
resolves them (which in this plugin's wording means "do the magic" of
canonicalizationm, whitelist checking and reducing to a configured
genre count).
- Clarify in _resolve docstring that a possible outcome might be all
genres being removed.
- Add an additional log message telling which existing genres are taken
into account BEFORE "the magic happens".
- Rename _to_delimited_genre_string() to _format_and_stringify()
- Move count reduction logic to _resolve_genres()
- Fix and rename a test
I found that the translator would sometimes replace the pipe character
with another symbol (maybe it got confused thinking the character is
part of the text?).
Added spaces around the pipe to make it more clear that it's definitely
the separator.
The return type of the stage decorator should in theory be `T|None`
but the return of task types is not consistent in its usage. Would need
some bigger changes for which I'm not ready at the moment.
URL-encode additional item `fields` within generated EXTM3U playlists instead of JSON-encoding them.
This is because JSON-encoding additional fields/attributes made it difficult to parse the `EXTINF` line but using URL-encoding for these values makes parsing easy (because URL-encoded values cannot contain commas, quotation marks and spaces).
I introduced the generation of additional EXTM3U item fields earlier this year and I want to correct that now.
**Design/definition background:**
Unfortunately, I didn't find a clear definition of how additional playlist item attributes should be encoded - apparently there is none.
Given that item URIs within an M3U playlist can be URL-encoded already, defining the values of additional attributes to be URL-encoded is consistent design.
I didn't find examples of additional EXTM3U item attributes in the web where the attribute value contains a comma, space or quotation mark but examples that specified numeric IDs and URLs as attribute values.
Because the URL attribute examples I found didn't contain URL-encoded characters and because it is more readable and unproblematic for parsing, I've let the attribute URL encoding treat `:` and `/` as safe characters.
**Breaking change:**
While this is a breaking change in theory, in practice it is not since afaik all integrations of the smartplaylist plugin's additional EXTM3U item attribute generation feature (beets-webm3u) work with simple attribute values such as the item ID (numeric) whose formatting/encoding is not affected when changing from JSON to URL-encoding.
In other words the change is backward-compatible with the beets-webm3u plugin (which I'll adjust correspondingly after this beets PR was merged).
Additionally, improve HTML pre-processing:
* Ensure a new line between blocks of lyrics text from letras.mus.br.
* Parse a missing last block of lyrics text from lacocinelle.net.
* Parse a missing last block of lyrics text from paroles.net.
* Fix encoding issues with AZLyrics by setting response encoding to
None, allowing `requests` to handle it.
* Type the response data that Google Custom Search API return.
* Exclude some 'letras.mus.br' pages that do not contain lyric.
* Exclude results from Musixmatch as we cannot access their pages.
* Improve parsing of the URL title:
- Handle long URL titles that get truncated (end with ellipsis) for
long searches
- Remove domains starting with 'www'
- Parse the title AND the artist. Previously this would only parse the
title, and fetch lyrics even when the artist did not match.
* Remove now redundant credits cleanup and checks for valid lyrics.
Tidy up 'Google.is_page_candidate' method and remove 'Google.sluggify'
method which was a duplicate of 'slug'.
Since 'GeniusFetchTest' only tested whether the artist name is cleaned
up (the rest of the functionality is patched), remove it and move its
test cases to the 'test_slug' test.
Having removed it I fuond that only the Genius lyrics changed: it had en
extra new line. Thus I defined a function 'collapse_newlines' which now
gets called for the Genius lyrics.
This commit introduces a distance threshold mechanism for the Genius and
Google backends.
- Create a new `SearchBackend` base class with a method `check_match`
that performs checking.
- Start using undocumented `dist_thresh` configuration option for good,
and mention it in the docs. This controls the maximum allowable
distance for matching artist and title names.
These changes aim to improve the accuracy of lyrics matching, especially
when there are slight variations in artist or title names, see #4791.
- Rename method _dedup_genre, since it's only used for
finalizing/polishing existing genres.
- Return separator-delimited string already.
- Decide on not passing "separator" to methods, it's a config
setting available throughout the plugin. Assign to variable where
useful for readability though.
- In the force branch, remove re-assigning keep_genres to empty list.
- Fix a test. Existing genres are "polished" now, which means:
configured title_case is applied.
- Fix/add type hints on all touched and new methods
- Adapt tests to _resolve_genres returning a list with not yet formatted genres.
- Rename and adapt test_count -> test_to_delimited_string. Note that the
new function does not apply whitelist, prefer anything. It just cuts
to count and formats!
- No idea where a missing separator (which is default) could
happen...just set it explicitely.
- Since we now refactored fetch_genre to returning a list we can add
mock multiple fetched gernes easier.
