fromfilename: improvements & fixes

[henry-oberholtzer]

This pull request improves the usability and configurability of the fromfilename plugin to encompass more file name patterns and folder names, which can contain usable information that will lead to better results when put through the metadata plugins, even for files with partial tags.

These changes gives the plugin the ability, by default, to extract the following fields:

album, artist, albumartist, disc, track, title, media, catalognum , year

A comprehensive set of tests has been written to make sure the patterns can recognize the information in a wide
variety of layouts and file names.

For more unusual folder and filename patterns, users can now specify a pattern, using the same method as our
path format strings, e.g. "$albumartist - $album ($comments)"

Changes & Issues Addressed:

• Full typing
• Increased test coverage with more edge case handling
• Make sure single files can also be checked for information
• Precompile regex expressions
• Expanded patterns from track information FromFilename Plugin don't work at all #5597
• Default patterns with better track index parsing, supporting disc and track combinations. fromfilename: Add filename matching regex for more forms of leading numerals #541
• Default behavior extracts information from the folder name (album artist, year, album title) fromfilename: Guess more fields from parent directory name #1554
• Allow users to contribute additional search patterns & fields fromfilename: Configurable extraction patterns #1400
• Configurable parent folder that can be ignored for imports
• Opted to not use a Consider configuration option to allow updating information when "Use-As-Is" is selected, as the intention of the plugin is to feed better data to the metadata plugins, but if it seems within scope, could add a menu command to "write found data". fromFilename Plugin: Does not update tags with AsIs actions. #5413~

Notes:

• Removed the "tag" field from the plugin, as its use and applications were vague, undocumented, and untested.

Housekeeping

• Documentation
• Changelog
• Tests

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 69.04%. Comparing base (afc26fa) to head (177e997).
⚠️ Report is 51 commits behind head on master.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6259      +/-   ##
==========================================
+ Coverage   68.22%   69.04%   +0.81%     
==========================================
  Files         138      138              
  Lines       18792    18736      -56     
  Branches     3167     3096      -71     
==========================================
+ Hits        12821    12936     +115     
+ Misses       5298     5149     -149     
+ Partials      673      651      -22     

Files with missing lines	Coverage Δ
beetsplug/fromfilename.py	100.00% <100.00%> (+7.35%)	⬆️

... and 11 files with indirect coverage changes

🚀 New features to boost your workflow:

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

[github-actions]

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.

[JOJ0]

just some tiny things I saw during a quick scroll through. this is a lot! but worth doing! thanks! I came along a bunch of old badly tagged mp3's lately that will be perfect for testing. hope I get to it.

[henry-oberholtzer]

I've been using it a good amount with my vinyl ripping workflow, and it's been working way better than the old version. If everything looks good & works well for you, I can update the changelog & get it merged in.

[JOJ0]

I've been using it a good amount with my vinyl ripping workflow, and it's been working way better than the old version. If everything looks good & works well for you, I can update the changelog & get it merged in.

I merged it into my personal master and want to check it alongside current discogs changes. I got distracted with a crash in Beets elsewhere though.... Will try to do some testing with badly tagged files as promised soonish.

One thing I wanted to ask: How did you come up with all the regex patterns? 1. By heart since I'm a regex wizard. 2. Had AI generate them. 3. Used regex101 (or similar) to develop and test them. :-)))

[JOJ0]

On a very high level, the first things I notice:

Given a badly tagged collection of Alternative Rock/Pop music residing in a folder named Alternative we find a hand full of single files as well as subfolders with artist names or even subfolders with full albums.

This might be out of scope with your suggested changes but since you are kind of the expert in refactoring this plugin at the moment I'd like to feedback anyway:

First beets tries to process the 7 files in the root of the collection as an album, thus I tell it to "Group Albums":

/......./Alternative (7 items)
...
...
fromfilename: Item updated with: dict_items([])
fromfilename: Item updated with: dict_items([])
fromfilename: Item updated with: dict_items([('album', 'Alternative')])
fromfilename: Item updated with: dict_items([('album', 'Alternative')])
fromfilename: Item updated with: dict_items([])
fromfilename: Item updated with: dict_items([('album', 'Alternative')])
fromfilename: Item updated with: dict_items([('album', 'Alternative')])
...
...
Enter search, enter Id, aBort, eDit, edit Candidates, plaY? g
...
...
fromfilename: Item updated with: dict_items([])
Looking up: /...Alternative/caesars+palace+-+jerk+it+out.mp3

In this case what does the "Item updated with " message mean exactly? Note that this single track actually does have an artist, album and title tag set.

On a next file i see:

In this case updating the item album Alternative is a bad idea since we just asked the importer to "Group Albums" and to not consider the containing folder's name as the album title (or I am mistaken and this is how Beets works even in "Group albums" mode. Let's investigate....).

fromfilename: Item updated with: dict_items([('album', 'Alternative')])
Looking up: /...Alternative/Elastica - Connection.mp3

This next one is probably impossible for the plugin, so we might just ignore that weird namings like that can't really be considered:

fromfilename: Item updated with: dict_items([('album', 'Alternative')])
Looking up: /.../Alternative/Stigmata Soundtrack - 01_Mary Mary (Stigmatic Mix) - Chumbaw.mp3

But also here we see that the missing album is replaced with the folder name which in this case doesn't improve the metadata search request. Also note that this file had a correct artist and title tag. Album was missing though.

/...Alternative/Stigmata Soundtrack - 01_Mary Mary (Stigmatic Mix) - Chumbaw.mp3 (1 items)
Sending event: import_task_before_choice
Sending event: before_choose_candidate

Finding tags for album "Chumbawamba - Alternative".
  Candidates:

I'm stopping now since I'm running out of time but will continue with this folder. There is loads of difficult material in it ;-)

Please don't take this too critical. Just trying to give a real life example and I'm sure there might be situations were no automation on this world will prevent a manual intervention, eg. search/rename/restructure.

One thing I always wanted to have in Beets is the ability to edit what is actually sent to the metadata source withihn the importer. This way an only halfway correct suggestion from filename or wonky tags could be quickly fixed. Similar to the edit plugin, but before the search, but this is out of scope here!

So the main thing I notice and question is: Is it expected that "non-album-folders" names will be considered as album names in "Group Albums" mode, and can we do anything about it?

Anyway, hope it helps, and again: Love that you took on the task of making this plugin more useful! I'll try to do more playing around this weekend I hope!!!

[henry-oberholtzer]

Woohoo! Thank you for this very thorough and constructive feedback. The rewrite was mainly done through my own lense, so there's a lot of scenarios I may have missed or things I didn't catch.

Regex was a combination of regex101 and by heart. I'm not gonna let a chatbot have all the fun writing regex!! I unfortunately(?) enjoy the process of writing out all the code myself, so unless someone else requires me to, I tend to stay away from LLMs with my code.

In this case what does the "Item updated with " message mean exactly? Note that this single track actually does have an artist, album and title tag set.

Good issue raised here - this log message could be more clear. It essentially shows the fields that it thinks are empty and it should substitute. Since all the files are lacking the album tag, it's using "Alternative" as the album tag.

One thing I always wanted to have in Beets is the ability to edit what is actually sent to the metadata source withihn the importer. This way an only halfway correct suggestion from filename or wonky tags could be quickly fixed. Similar to the edit plugin, but before the search, but this is out of scope here!

This would be so useful! Maybe it could be an expansion of the edit plugin, since it's sort of on the right track, and could stand to allow more fields to be edited too.

Here are the changes I think I 'll make to have it perform better that will hopefully

1.I think I will restructure that log message to the following:

fromefilename: guessing "filename.mp3" album="Alternative"

This should be more instructive as to what it is actually doing with each file.

2. I'll also make sure it respects the group album command or doesn't try to update the album field if something else in the group has an album tag already. I'll also see if it can check to see if the --group-albums flag is thrown and then disable the parent folder guess by default.

3. I think adding just a configuration toggle to not guess the parent folder would be a good option, in addition to the ignore_folder command.

Thanks for this great feedback - would much rather get this right the first time than have to keep patching it.

[JOJ0]

Regex was a combination of regex101 and by heart. I'm not gonna let a chatbot have all the fun writing regex!! I unfortunately(?) enjoy the process of writing out all the code myself, so unless someone else requires me to, I tend to stay away from LLMs with my code.

Haha! Yes! That's the spirit! I'm lazy often I have to admit and let AI help me, but especially with regex I had bad experiences and regex101 and human thinking is the only way! Brings back one thought I'd like to throw in in case you are not aware already: Putting more complex regex into yaml can be a PITA, which is the case here with the custom patterns you implemented. If people try to do crazy black magic with this feature we might get issues in the future. Didn't look how you are escaping these patterns. Will do in my final low level review but I'll let you continue first withe below's awesome feature ideas! ❤️

One thing I always wanted to have in Beets is the ability to edit what is actually sent to the metadata source withihn the importer. This way an only halfway correct suggestion from filename or wonky tags could be quickly fixed. Similar to the edit plugin, but before the search, but this is out of scope here!

This would be so useful! Maybe it could be an expansion of the edit plugin, since it's sort of on the right track, and could stand to allow more fields to be edited too.

Let's definitely keep that in mind for the future! Yeah an edit plugin PR sounds doable!

Here are the changes I think I 'll make to have it perform better that will hopefully

1.I think I will restructure that log message to the following:

fromefilename: guessing "filename.mp3" album="Alternative"

This should be more instructive as to what it is actually doing with each file.

That sounds super clear, explains everything and is still quite concise! ❤️

2. I'll also make sure it respects the group album command or doesn't try to update the album field if something else in the group has an album tag already. I'll also see if it can check to see if the --group-albums flag is thrown and then disable the parent folder guess by default.

Please help me out here: When we set it on the CLI already it will be enabled throughout the entire import run.? No matter in which folder a file resides, the importer will always try to mere everything together right?

Now if I enable it interactively during the importer run (like in my example), only for this one folder (and it's subfolders???), will the group albums feature unset itself when we jump out of the folder again. Eg. in my example, once we cd .. from Alternative/ again and move on with the next top level folder.

Or will it stay enabled always from that point on, in the import run? Sorry, confused how Beets actually works in that matter 🤷

3. I think adding just a configuration toggle to not guess the parent folder would be a good option, in addition to the ignore_folder command.

Hmm if that is easy to implement, do it. Also add a CLI option for it then. I'm picturing a workflow where you beet import .... and point it to a folder where you are sure is not sorted in folders by albums but only for example with artist names folders Aritst1/, Artist2/. Having to reconfigure sounds tedious, better quickly switching mode via CLI option sounds more user friendly.

What I missed in my first read-through is that you actually already thought of disabling this in a way:

https://github.com/beetbox/beets/pull/6259/changes#diff-dc32f40dc487a7e101c9189fedc8269826b4d92300b58c8646037a7be24cf51eR84-R86

So with

in addition to the ignore_folder command ...

you tried to point to this feature? Right?

This is a great addition, let's try not ot overengineer it but brainstorm a little. How would a CLI option look like that tells the importer (well actually the fromfilename plugin) that for this run the guessing by foldernames for albumnames shouldn't be done. Something like beet import --no-fromfilename-folder-guess sounds a bit superweird. I'll think about this more....drink more coffee first... ;-)

Thanks for this great feedback - would much rather get this right the first time than have to keep patching it.

Dozens of thanks going out here! This will be so so super helpful!