Resolves#5027: failing test: test_nonexistant_db
This test was failing because it was requesting input from the user on stdin.
This diff mocks stdin with a canned response.
Instead of restoring `sys.stdin` to `sys.__stdin__`, restore it to
whatever it was before we installed out dummy I/O hooks. This is
relevant in pytest, for example, which installs its *own* `sys.stdin`,
which we were then clobbering. This was leading to the suppression of
test failures observed in #5021 and addressed in #5027.
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
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
Fixes#5649, more precisely the behavior described in comment
https://github.com/beetbox/beets/issues/5649#issuecomment-2732245358
where a last.fm genre was found for the album but `_resolve_genre()`
kicked it out because it was't allowed by the whitelist which resulted
in writing an empty genre as the final result.
- This fix makes sure that when no album genre is found the next stage
is hit - returning with either the original genre (if any), the
configured fallback genre or None.
- A new option `--debug` or configuration setting `extended_debug: yes`
can be set to see precisely what tags were fetched during the track,
album or artist stages prior to filtering by a whitelist or
canonicalization tree. It's not enabled by default since it clutters up
the debug log quite a bit especially during *Various Artists* checks,
trying to get the most popular track genre.
Some refactorings to make the plugins behavior better readable and also
fixing some regressions:
- `_resolve_genres` returns a list and so does `_combine_genres` (which
is now renamed to `combine_resolve_and_log` which more precisely
reflects what it's doing).
- _`to_delimited_genre_string` included reducing to count *prevsiously*,
which was kind of hidden in there and not obvious on first sight and
also it did some formatting. The name was bad! It is now called
`format_and_stringify` (the count reduction part moved to
`_resolve_genres`)
- Since `_resolve_genre` does count-reduction when canonicalization is
configured, it makes sense to also do that when only whitelist checks
are done.
- New log message introduced in `_combine_resolve_and_log`showing the
list of existing genres that are taken into account before
whitelist/canonicalization does its magic.
- `_resolve_genre` got a docstring describing what it's doing
- New helper `_filter_valid_genres` now used in `_resolve_genre`
(instead of hardcoded is-valid-list-comp) and other places
(`fetch_genre`)
- `fetch_genre` now also does a valid check. That way it is assured that
if everything was kicked, the next stage is entered (eg. album ->
artist)
- This fallback to next stage is now tested with a new `test_get_genre`
test-case.
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.
- Revert/fix last.fm fetcher methods to validate genres.
- In past versions (<=2.2) _resolve_genres which included whitelist
checks ran instantly after fetching last.fm tags which made sure the
next stage is hit when nothing worthwhile was found (e.g fallback
album -> artist).
- Bring back this behavior but don't run a full _resolve_genres but a
quick valid (whitelist) check only!
- Introduce an extended config/CLI option that allows to really log what
each stage fetches (prior to validation/whitelist filtering).
- Since this potentially is verbose especially with VA albums (a lot
of artist tag fetches) for performance and debug log clutter reasons
this is disabled by default.
- Clarify final last.fm tags debug log message to "valid last.fm genres"
which was the usual behaviour in lastgenre ever since and it should be
kept that way. Also refactor "if track" to use a similar notation for
overall code readability.
- 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
As reported in #5649 when new last.fm genres were found, they still might
get kicked out by the whitelist check in _resolve_genres(). This might
lead to _combine_genres() returning an empty list.
The desired outcome though is that since still nothing worthwhile was
found, the next stage should be entered - which in this case is,
returning with the configured fallback genre (or the default fallback
None).
The any() check makes sure this is the case and moving out the
string conversion from _combine_genres() makes this code slightly more
readable.
Typing Extensions is an optional requirement for `python>3.10`, thus
TypeVarTuple and Unpack need another import in higher python versions as
`typing_extensions` is not available
see https://docs.python.org/3.10/library/typing.html for supported types
close#5695
Bump python version in the CI to current versions.
This is the same as #5503 that for some reason got closed when the merge
of the `drop-py38` branch happened
Fix ocurrence of `UnboundLocalError` in plugins/listenbrainz >
`get_tracks_from_listens()` when `mbid` is not available.
Removed a print statment.
Fix link to config.yaml.
Fix link to Listenbrainz "get the token" documentation.
Co-authored-by: Šarūnas Nejus <snejus@protonmail.com>
## Description
My library is managed using Beets for organization and
[git-annex](https://git-annex.branchable.com/) as storage backend.
Therefore when using this system, while my library files always exists
on my filesystem, some files may be empty (without content). In this
case, when I'm running the `convert` plugin, I don't wants it to process
files which are empty (same apply for any Beets plugin). Hence, I added
a check that the file is readable as a `MediaFile` before doing any
process.
Before this fix, trying to encode an empty file would have lead to an
error while leaving `convert` doing its side-effects **and** `convert`
would also copy empty files to destination for files that doesn't need
to be re-encoded.
In my case, this is empty files, but the problem can be anything else
(depending on the storage backend) and/or corrupted files. Conclusion, I
think **checking that the file is readable is always recommended before
proceeding to heavy operation** like this.
🎵 The Refactoring Blues 🎵 by [Claude](https://claude.ai)
Verse 1:
Got those lyrics plugin blues
Cleaning up some messy code
Moving classes, fixing views
Making changes down the road
Chorus:
We're refactoring tonight
Making the codebase clean and bright
Translation's got a brand new home
And ReST files found their own
Verse 2:
Added Microsoft Translate
Keeping tokens safe and sound
Config options up-to-date
Better structure all around
Bridge:
Path operations simplified
Groups of artists, neat and tied
Error handling's looking fine
Comments clear along each line
Verse 3:
RestFiles in their own class now
Cleaning imports, showing how
Better typing makes it clear
What should go and what stays here
Final Chorus:
We're refactoring tonight
Making the codebase clean and bright
Translation's got a brand new home
And our code can stand alone!
— inspired by _the diff_
---
### Technical Changes
- Replaced deprecated and broken Bing translations by Microsoft
Translator API
- Isolated all functionality in the `Translator` class.
- Updated translation settings configuration.
- Added support for synced lyrics from LRCLib.
- Added support for preserving existing translations to help users to
manage their characters quota.
- Added error handling and logging
- Added tests
- Created RestFiles class for ReST document handling
- Simplified path operations using pathlib
- Added tests
- Improved command line options handling
#### Caching of translations
The plugin will not re-translate lyrics if translations already exist,
see
```fish
$ beet -v lyrics albumartist::Sel karta -f
...
lyrics: LyricsPlugin: Fetching lyrics for Sel - Saulės Miestas
lyrics: LRCLib: Fetching JSON from https://lrclib.net/api/get
lyrics: LyricsPlugin: 🟢 Found lyrics: 32275 | 1996 / Neįvertinta Karta: Sel - Saulės Miestas
lyrics: Translator: Posting data to https://api.cognitive.microsofttranslator.com/translate
lyrics: Translator: 🟢 Translated lyrics to EN
$ beet -v lyrics albumartist::Sel karta -f
...
lyrics: LyricsPlugin: Fetching lyrics for Sel - Saulės Miestas
lyrics: LRCLib: Fetching JSON from https://lrclib.net/api/get
lyrics: LyricsPlugin: 🟢 Found lyrics: 32275 | 1996 / Neįvertinta Karta: Sel - Saulės Miestas
lyrics: Translator: 🔵 Translations already exist
```
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.
## Description
Hello y'all.
One of the dev from the [beets-flask
app](https://github.com/pSpitzner/beets-flask) here. We are extending
the `ImportSession` in our application to get the interactive imports
working in our application. Overall that was not a too pleasant
experience as a ton of typehints are missing in that area of the beets
code. This PR wants to fix this ;)
### Changes
Moved importer state logic into a dataclass. Is clearer and more
isolated now in my opinion, we are not using it but I found it quite
confusing when looking at it.
Added a ton of typehints to the `importer.py` file. Predominantly the
`ImportSession`, `ImportTask` (and derivatives) and pipeline stage
decorators are now (hopefully) completely typed. Additionally I fixed
some typhint issues in the `util.__init__.py` file, mainly related to
the `PathLike` type.
If you have some strong opinions about any of the changes feel free to
revert or edit the PR.
Best,
Sebastian
P.S: I noticed that beets does not use Abstract Base Classes is there
any reason for that or is it just of historic nature? I think some Base
Classes could be adapted to use an ABC metaclass quite easily.
