Set field values on the import command line

[t-8ch]

Usage: beet import --explicit-field context=party --explicit-field category=disco some_folder

I have different categories of music which I don't want to mix. For this is would be useful to be able to specify explicit metadata on import.

[sampsyo]

Thanks! I'm in favor of this feature. I could have sworn we'd discussed this before, but I can't find any other issues about it (#565 discusses something vaguely similar). Perhaps it was on the mailing list?

[t-8ch]

I never read the ML, so I have no clue whether it has been been proposed before. Maybe #488?

[sampsyo]

Yeah, that's also related. Well, I'm glad we have an issue now!

[acastaner]

I would totally love this feature too. I have thousands of artists and the way beets stores by default is great, but generates a ton of directories (one for each artist). This makes it impractical (for me anyway) to look for media when browsing, and causes long delays over network shares (CIFS sends all the folder names at once).

Being able to arbitrarily split the library some more (by genre or otherwise) allows to break this down more and I feel this is critical for people with a large library.

[Kraymer]

Being able to arbitrarily split the library some more (by genre or otherwise) allows to break this down more and I feel this is critical for people with a large library.

@acastaner any existing metadata (not just artist) can be used for path configuration, cf https://beets.readthedocs.org/en/v1.3.17/reference/config.html#path-format-configuration

bucket plugin can be of some use to resolve your problem of network delay due to huge folder listings.

[acastaner]

Mostly my need (and what this feature request is about) is to set the field value upon import. This way it's not required to "pre-tag" the library we want to import, or manually tag after import.

For instance my beets is setup so that anything with the "savedir" field with value "metal" goes to the "metal_hard-rock" directory. But beets will act on this only through the mod command, not import. So I have to import everything then manually beet mod all the files. Not practical. If we could do it upon import then that issue is solved because the operation happens onthe specified directory not through filters/queries.

I hope I'm clear and making sense :-)

On Sun, Apr 17, 2016 at 1:40 PM -0700, "Fabrice Laporte" notifications@github.com wrote:

Being able to arbitrarily split the library some more (by genre or otherwise) allows to break this down more and I feel this is critical for people with a large library.

@acastaner any existing metadata (not just artist) can be used for path configuration, cf https://beets.readthedocs.org/en/v1.3.17/reference/config.html#path-format-configuration

bucket plugin can be of some use to resolve your problem of network delay due to huge folder listings.

---

You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#1881 (comment)

[tissieres]

Hi,
I'm interested in this feature as well and willing to help. How can we advance ? Do you know where we should start ? I'm not very familiar with the internals of beets.

[sampsyo]

Awesome! I'm glad you're interested in helping.

The right place to start is with the configuration handling in importer.py. You'll probably want to get familiar with the codebase in general first, for which the best place to start is probably the "Hacking" page in the wiki. Can you take a look at that and browse through the importer module first? Then come back and ask questions about how things work in general.

Eventually, the feature should probably work via a configuration option that adds the attribute in the importer stage that applies the metadata from a match.

[tissieres]

I suggest to implement this as follows:

First, we add an optional argument to import that takes multiple field=value arguments like this:
beet import --force-field context=party category=disco some_folder
The behaviour can be somewhat copied from modify command.
Then, we implement a new function in importer.py that will be called at the end of ImportTask.apply_metadata() and SingletonImportTask.apply_metadata() and that overwrites selected fields that are found in config.
Does it make sense? Do you think it is better to handle this directly in autotag module?

[sampsyo]

Yes; this sounds great! I completely agree it should live in the importer module, not autotag. And apply_metadata is definitely the right place to do it.

Details on the interface:

• I might call the command-line option --set or --set-field or --field, since "forcing" doesn't make too much sense in the case that you're inventing a new flexible attribute.
• As with the rest of the importer command-line flags, this will be backed by a configuration option that will show up in ImportSession.config. The configuration option can have the same name as the command-line flag.

[acastaner]

Just wondering how's the progress @tissieres ? Really just can't wait for that feature! I don't really write Python but I could help you test if you need

[tissieres]

Did not have much time to work on it, but it's still on my todo list...

[sentriz]

I am 100℅ in favour of this. Some of my music has a synced tag. Everything with that tag gets put in a folder that's synced to my phone. So with this setup, if I wanted a new album on my phone, I just beet import --field synced=True folder/ instead of importing normally and running beet mod

[bartkl]

@sampsyo I have begun trying to implement the ability to add options to subcommands via Plugins as mentioned in #2567. It seems to work, but I want to make sure I'm not underestimating things here, especially since you mentioned the probable need for a new CLI parsing library which I didn't use.

Basically what I did is:

1. In the _setup function in __init__.py all subcommands are initialized, and immediately after that I create a dictionary subcommands_dict from it (of which the keys are the Subcommand.name properties) which I pass to the new add_additional_options function of plugins.py. This is done in a try/except block because of potential conflicting option errors.
2. This new add_additional_options(subcommands_dict) function in plugins.py looks for add_additional_options methods in all plugins returned by find_plugins() and calls these methods, passing on the aforementioned subcommands dictionary.
3. Plugin developers can therefore write such a method add_additional_options(subcommands) in which they can alter the Subcommand objects passed (by object reference, thank you Python, and therefore returning won't be necessary) in the subcommands dictionary. This way developers can easily access the subcommand objects using their CLI name, like so: subcommands['import'].parser.add_option(u'--test-me), or assign a local variable to it for ease of use, like import_cmd = subcommands['import'] and carry on from there.

At a first glance this seems to work. Could you please give me some feedback? I'm a little insecure because of my lack of experience, so this feels like a great oppertunity to learn!

If you think this sounds good, I'm going to look into part two of the story: accessing those new CLI options/arguments of these subcommands from a plugin, so they can be put to use instead of simply being CLI-candy (see what I did there? ;)).

[sampsyo]

Thanks for looking into this. Regardless of the specific design, though (which looks good!), I'm still a little nervous about the fundamentals: this basically ties us to use optparse forever, instead of an eventual switch to Click or another, more modern option-parsing library. (See #2190 and #1484, for example.) Giving all plugins access directly to the opts value for each command also seems like it could add a lot of complexity—what happens when plugins start wanting to extend commands provided by other plugins, for example?

Is there any chance we should just direct this effort toward the original goal of this ticket, which is just one new flag that lets you set fields for the import command? That would avoid the complicated issues of extensibility and directly support what people seem to want, which is just setting an arbitrary field without any plugin support.

[bartkl]

Thanks for your feedback. And funny you should mention: I actually tested adding custom options to subcommands provided by plugins, because I was also a bit weary about it. Anyhow, I was unaware of the longterm plans of phasing out optparse, so then obviously this path is sub-optimal at best.

Good point, I'll look into that instead. Again thanks for your feedback, this is really educational for me :).

[bartkl]

Yes; this sounds great! I completely agree it should live in the importer module, not autotag. And apply_metadata is definitely the right place to do it.

Actually, if my digging is accurate, it seems not be a suitable place after all. The apply_metadata method is only called if the choice is apply, which is not the case if autotagging is disabled. See:

    if task.apply:
        task.apply_metadata()
        plugins.send('import_task_apply', session=session, task=task)

I verified this by noticing that when I passed -A on the command line, it never emits an import_task_apply event, which as you can see depends on the same condition. Nowhere else is apply_metadata called, so implementing this functionality there will effectively disable it for as-is import processes.

How about an extra stage, say set_fields, for this, which is appended to the pipeline in run() (depending on the config value)? I'm thinking out loud, have not entirely grasped the mechanics here, but my guts tell me this is worth suggesting.

PS My guess for the reason why it imports the metadata anyways is because of the transaction at the bottom of the manipulate_files method, whose intended task is probably to only update the path values, but I suspect that's the place where in the case of not autotagging the entire items are updated. It's also the only stage left. I am puzzled still though, since only items are updated, so what about album fields? I'm just thinking out loud here to get clarity.

[sampsyo]

Aha, good point. You're right that apply_metadata won't quite do for imports that wouldn't otherwise apply metadata.

An additional import stage would totally work; or we could tie the update onto the "finalize" stage. (You're right that the file-manipulation stage updates the database just to store new paths when the copying/moving has finished, which won't affect album structures.)

As alternative, a more complex design could avoid extra transactions by changing the metadata before it ever reaches the database in non-autotagged mode, or by changing it right after applying metadata when using a match, or by falling back to a separate transaction when the user chooses "as-is". But that's probably premature optimization. 😃

[bartkl]

Regarding the manipulate_files stage: so when importing as-is, this is currently where all item metadata is synced to the database intentionally? Or is the intention to only sync the updated paths there? Because it is (I believe) where all metadata syncing (for items) takes place right now. And where is the album metadata synced then in this scenario with no autotagging? (Or is this inferred by triggers in the database or something when items are updated?)

Personally I think choosing to add this to the finalize stage is concealing and sacrifices separation of concerns for simplicity. An additional import stage feels neater, wouldn't you say?

I'm not sure I follow entirely. One thing for me to grasp here I think is: is it desired to sync to the db after updating the models, or should this be taken care of by the transaction near the end of the import process (in file manipulation stage for instance, which kind of repeats my first question as to the intended behavior of its transaction)? I'm confused when syncs to the db occur, it doesn't seem to be consistently after each metadata operation, nor after all of them in a single sync (which could reside in finalize). Maybe you could shed a little light on this for me.

Thank you!

[bartkl]

It's coming along nicely! Parsing the arbitrary amount of field/value pairs like --set-field x=a y=b is hard though. The idea to borrow from the modify command is not going to suffice: in the modify case the indefinite amount of key/value pairs and queries are all after the options, i.e. at the end of the args. Other options could follow it here, and on top of that the nargs of optparse only accepts a fixed number of arguments to consume, so that won't be of any help either.

Am I missing a way of doing this here?

1. Otherwise could we go with: --set-field x=a --set-field y=b? I had that working just now.
2. Another idea could be to make it a single value, but separated by a delimiter like a comma: --set-field x=a,y=b``. This approach looks nice, but has one of the two following disadvantages: a. A nice, intuitive delimiter like ,` could actually be character within the key/value pairs, and you don't want to bother the user work with escape characters or field delimiters in a non-standard way.
   b. A safer delimiter like '|', but this makes everything a little cryptic in my opinion.

I'd go with method 1, but be sure to let me know your ideas!

[sampsyo]

Regarding the manipulate_files stage: so when importing as-is, this is currently where all item metadata is synced to the database intentionally? Or is the intention to only sync the updated paths there? Because it is (I believe) where all metadata syncing (for items) takes place right now.

Yeah, the intention is just to save the new paths there. Of course, we don't currently have a way to update one field at a time, so it has the side effect of syncing all the metadata.

Other metadata actually has other places where it's synced. First, all the initial metadata for all imports is added to the database in apply_choice (in the call to add), which is called both from import_asis (for non-autotagged imports) and user_query (for interactive imports). Then, each plugin is responsible for syncing any changes it makes, just like the manipulate_files stage is responsible for saving its updates to the paths.

So, I totally agree that finalize is the wrong place, but how about apply_choice? That way, we only have to touch one function, and it's actually the logical place to set the proper metadata for imported music. ✨

[sampsyo]

And yeah, good point about the complexity of parsing the CLI options. I somewhat strongly agree with out about preferring this option:

Otherwise could we go with: --set-field x=a --set-field y=b? I had that working just now.

In addition to being easier to parse, I also like it better from a UI perspective! It avoids violating people's expectation that flags look like --foo bar --baz qux, while in --foo bar baz, the final word is a "positional" argument. (And we don't have to invent a new delimiter.)

[bartkl]

Ah, of course, I overlooked the fact that add does in fact sync with the db, it's clear to me now, thanks.

apply_choice sounds like a very good place indeed. Does this have your preference over having a new stage right after it, i.e. between the initial import (autotag/as-is) and the plugins stage? I have that working now, using a pipeline.set_mutator_stage decorator, although I have to admit I'm not entirely sure how that decorator and the pipeline specifics work (yet), but since it works and it's going to be reviewed anyways maybe it's fine. Let me know.

Another thing I bumped into: should field/value pairs supplied at the CLI be appended to the ones specified in the config block, or override them? In the case of appending it's especially interesting if you define the same field in both places, in which case the user might expect both values to be aggregated and written as a multivalue (even though those aren't supported yet, right?), but in fact the CLI value will overwrite the config one.

Finally: I've read the "Testing" section in the wiki, but I think I need some help. Is the IRC channel a good place to look?

Sorry for all the questions; it's my first open-source contribution.
Thanks!

[sampsyo]

Yeah, I think I do like the idea of using apply_choice is a bit better: I think the new code would be the most discoverable if we put it there.

That's a good question about the config/CLI composition. I suppose I don't have a strong opinion—but perhaps the most logical thing would be to append the CLI settings onto the config settings, so the user does have the option to override the config if they want to.

Yeah! IRC (or Discourse, or here) is absolutely a good idea for questions about the tests.

And no problem! Answering questions is what we're here for.

[bartkl]

Okay, I think I'm done with everything except the tests. I asked help via IRC and I think I know where to start. I still have a few questions though:

1. I'm not sure to what length I should go writing tests. Testing unicode characters sounds reasonable, but should I test if it works fine if empty values are supplied for instance? I'm not sure what to test.
2. I figure these tests belong to the test_importer test suite, but I'm not sure where to start there. I can imagine implementing the test in both ImportSingletonTest and ImportTest makes sense, but I wouldn't know if I have to implement tests in other classes (what about NonAutotaggedImportTest for instance?). And then: depending on how many test cases there are, should I create a nested class as a 'grouping wrapper'?
3. Or should I subclass unittest.TestCase and start working from there? In that case: can I access mock imports and the likes (and if so, what would be a nice example to borrom from?)

And as far as checking for valid input goes, I could implement a custom optparse type named something like key_value_pair and that way force the input to be of the form key=value. Would that be a nice way to handle this, or overkill (it's not hard though). Another option is to catch an exception that occurs because of it (most likely when store() is called). I would like to hear your ideas.

Thanks again.

[sampsyo]

Great! I'm glad IRC was helpful. If you learned anything that you think would also help other newcomers, would you mind adding it to the testing wiki page? That would be awesome!

• In my view, you can start simple. Just a couple of "obvious" tests to show that the option does what it's supposed to will be totally fine. From there, we can worry about more complicated stuff like Unicode and edge cases like empty strings.
• Adding one test to ImportTest and one to SingletonImportTest seems like a great way to get started.

For validation, I think a reasonable strategy would just split on =, and if there's no = in the string, just raise a UserError (out exception for user-visible errors). It might even be useful to reuse the parsing logic from the modify command if that seems feasible!

[bartkl]

I believe I'm done :). It seems to work fine!
I'm going to create a pull request now, and hope for the best. I take it that if stuff isn't okay yet it will be reviewed and discussed there?

[sampsyo]

Great! Yes! I'll leave comments on the pull request.

[sampsyo]

Fixed by @bartkl in #2581. ✨ 🎉 🐟

[t-8ch]

Thanks @bartkl!

[acastaner]

Is the final command --set-field x=y (as detailed in this discussion) or --set x=y (as detailed in the release notes) ?

E: It's as per the release notes, just checked better :)

[acastaner]

So I've just tried this feature in details in beets 1.4.5 and it doesn't seem to behave completely as I expected (could be that my expectation was wrong). Maybe my usage is a bit of corner case.

I like to sort my music by directories named after the style of music in them. So I have "ost", "classical", "metal" and so on. In order to tell beets where to move the files I setup a specific configuration in config.yaml :

paths:
    albumtype:soundtrack: ost/$album/$disc_and_track. $title
    savedir:ost: ost/$album/$disc_and_track. $title
    savedir:arthur: _arthur/$album/$disc_and_track. $title
    savedir:radionova: nova/$album/$disc_and_track. $artist - $title
    savedir:world: world/$albumartist/$year - $album/$disc_and_track. $title
    savedir:electro: electro/$albumartist/$year - $album/$disc_and_track. $title
    savedir:classical: classical/$artist/$album/$disc_and_track. $title
    savedir:alt_rock: alt_rock/$albumartist/$year - $album/$disc_and_track. $title
    savedir:rock: rock/$albumartist/$year - $album/$disc_and_track. $title
    savedir:jazz: jazz/$albumartist/$year - $album/$disc_and_track. $title
    savedir:blues: blues/$albumartist/$year - $album/$disc_and_track. $title
    savedir:metal: metal/$albumartist/$year - $album/$disc_and_track. $title
    savedir:rap: rap/$albumartist/$year - $album/$disc_and_track. $title
    savedir:pop: pop/$albumartist/$year - $album/$disc_and_track. $title
    savedir:techno: techno/$albumartist/$year - $album/$disc_and_track. $title
    default: _trier/$albumartist/$year - $album/$disc_and_track. $title
    comp: $genre/$album/$track. $title

So (mostly) based on the "savedir" flexible attribute beets knows where to put my music. So far I was settings this by hand using beet mod but when dealing with a larger amount of music it's not very practical.

Now that my use case is hopefully clear I should explain my expectation. I was expecting that when doing something like beet import 'music' --set savedir=pop not only would beet import the music (and apply all the replayGain or acoustid options I have) but also hit the right "path" config (pop in this case) and be moved there.

But this is not what happens ; beets imports fine but doesn't move the files. It does however add the flexible attribute, so when I try to manually trigger the change like before by using beet mod then beets tell me there's no change to be made :)

Is there a way to change this behavior? Am I in some kind of edge case?

[sampsyo]

Hmm! We should totally support this use case; it's clearly part of the utility of the new option.

I can imagine two problems: paths are determined before we set the fields; or some problem with album-level vs. item-level fields. To check this, can you see whether running beet move on the newly-imported items has the effect you want?

[Vrihub]

Hmm! We should totally support this use case

I'm interested in this case too. But I was wondering: can't one just use $savedir as a variable in the path definition, instead of listing all the different cases? Something like:

paths:
  default: .../$savedir/$albumartist/$year - $album

[quizilkend]

Hey there,
I would love to use this, but unfortunately it isn't working as expected.
When I import a folder with:
beet import --set context="Kinder" /media/Medien/Musik/Kinderlieder/Muckemacher/
It imports the folder but doesn't add the context=Kinder field.

Am I doing something wrong?

[t-8ch]

The fields are set on the album, not the single tracks inside it.

[quizilkend]

Thank you, I didn't know that! Can I use them like the fields in advanced awesomeness?
I have the same usecase as acastaner in this thread, and would be willing to execute beet move after the import to move the files to the right location.

[t-8ch]

No clue. But even if not, you should manually apply your album-tags to contained tracks and then use those values.

[A-Costa]

Hey there, i know this is closed, but wouldnt it be nice to have --set actually work on tracks fields instead of just albums?

[wisp3rwind]

Hey there, i know this is closed, but wouldnt it be nice to have --set actually work on tracks fields instead of just albums?

What are you trying to achieve? Might that already be solved by #2988?

Otherwise, if there's still a use-case for this, I guess someone would need to come up with a design for how to build the CLI and configuration for setting album-fields.