Query parsing: numeric flex attributes

Do you agree with me that this is unexpected behaviour?:

$ beet ls mood_happy:0.5..1 -f 

I get no results and by examining the query I see that the above query was parsed to:

AndQuery([SubstringQuery('mood_happy', '0.5..1', False)])

By hacking into the parser and making it yield this:

AndQuery([NumericQuery('mood_happy', '0.5..1', False)])

Everything works as expected.

I am working on the beets-goingrunning plugin which makes extensive use of these numeric flex attributes. In the plugin I created a custom parser which is based on a hardcoded list of attribute names used to indicate for which attributes should the NumericQuery be used.

I don’t really like the idea of the hardcoded list (I might move it to the plugin’s configuration) but it does the job for now. Do you have any better ideas? Also, could this be something to be integrated into the core parser?

You’ll want to mark your field mood_happy with an integer type:
https://beets.readthedocs.io/en/stable/dev/plugins.html#flexible-field-types

Then the query parsing will work as you expect it to.

WooooW! You guys have thought of everything :astonished:!

1 Like

It works like a charm (I just cut out 200 lines of code). :smiley:

1 Like

:cry: but it makes my plugin incompatible with acousticbrainz…

Traceback (most recent call last):
  File "/Users/jackisback/opt/miniconda3/bin/beet", line 8, in <module>
    sys.exit(main())
  File "/Users/jackisback/opt/miniconda3/lib/python3.7/site-packages/beets/ui/__init__.py", line 1266, in main
    _raw_main(args)
  File "/Users/jackisback/opt/miniconda3/lib/python3.7/site-packages/beets/ui/__init__.py", line 1249, in _raw_main
    subcommands, plugins, lib = _setup(options, lib)
  File "/Users/jackisback/opt/miniconda3/lib/python3.7/site-packages/beets/ui/__init__.py", line 1148, in _setup
    library.Item._types.update(plugins.types(library.Item))
  File "/Users/jackisback/opt/miniconda3/lib/python3.7/site-packages/beets/plugins.py", line 344, in types
    u'another type.'.format(plugin.name, field)
beets.plugins.PluginConflictException: Plugin goingrunning defines flexible field danceable which has already been defined with another type.

Do you have any ideas how it could be solved whilst keeping both plugins active?

Actually, it looks like a bug.

The goingrunning plugin and the acousctibrainz plugin define the danceable attribute with the exact same type: 'danceable': types.Float(6).

Looking at the check here:

the plugin_types[field] != types[field] is comparing the two instances and not their types. Of course they are different. In fact, this:

if field in types and type(plugin_types[field]) != type(types[field]):

works.

What do you think? Should I open an Issue for this?

Hi! Yes, please do open an issue for this! It would be great to fix it.

However, the right fix is unfortunately not to add type(...) calls. We want to compare the objects, which represent beets data types—not the types of those types, which are Python classes. (I know it’s confusing.) The problem is that the type objects need to define __eq__ or __cmp__ in order to be comparable.

Ok, let’s do it! I’ll create the issue and have a go at the fix. I think I understand what you mean by comparing not the built-in types but the classes those instances created from. I never used __eq__ or __cmp__ but there is always a first time.

I am also thinking about proposing a different way beet handles this issue. At the moment it exits with a PluginConflictException. What if we load the plugins in the order they are indicated in the configuration file and we adopt a “first plugin wins” strategy with only a warning when fields are redeclared with a different type? What do you think?

have a great day

My plugin suffers of this issue only in one specific point. That is when I parse the query string composed from the command line queries + those picked up from the configuration file for a specific selection. If I do not override the types (the way you showed me) the parsed query will be wrong (for my needs). If I do the application will crash because of the bug mentioned above. So, this would be a no-go situation.

However, I really want to create a new release of my plugin and I want it to work. And I also want it to play nicely along with the acousticbrainz plugin and potentially any other plugin that might declare these attributes.

So, this is what I came up with:

    def _retrieve_library_items(self, training: Subview):
        full_query = self._gather_query_elements(training)

        # Store a copy of defined types and update them with our own overrides
        orig_types = library.Item._types.copy()
        override_types = common.get_item_attribute_type_overrides()
        library.Item._types.update(override_types)

        # Execute the query parsing (this will use our own overrides)
        parsed_query = parse_query_string(" ".join(full_query), Item)[0]

        # restore the original types
        library.Item._types = orig_types.copy()

        self._say("Song selection query: {}".format(parsed_query), log_only=True)
        return self.lib.items(parsed_query)

I don’t think it needs much explanation. It’s a hack but it works great. It works with AB plugin activated and the query gets parsed correctly.

Obviously I do not have the insight into Beets as you do.
Do you see any issues with this solution?

1 Like

Seems pretty good for now! Of course, the real fix will be in beets itself (or in just disabling one or the other plugin for now), but this should do in the interim.

1 Like

I should also have mentioned: it should work to let one plugin specify the type. That is, if you have the acousticbrainz plugin enabled and it declares the types for all the fields you care about, that should be enough—it shouldn’t be necessary to also declare the same type in your plugin. (Of course, that would prevent people who only want to use one plugin from taking advantage of the types.)

Oops, and I missed this one above:

To be honest, I think we should not do this. It would make things more confusing for end users—they would be able to see that one plugin declares a field with type A, and another plugin makes it have type B, and it would be unclear why one plugin “succeeded” in this declaration and the other “failed.” I think it’s better for the plugins to just resolve their conflict and avoid declaring differing types, hence the ugly exception.

Sure, that totally makes sense. But my plugin can not depend on what the AB plugin declares or on how it declares them. Also, many people will not have the AB plugin enabled so I do need to have my own implementation that works.

At one point I also thought about checking if the AB plugin was loaded and only when it was not I declared my fields. It also worked. However, the ‘hack’ solution should be compatible also with other plugins not only AB.

:+1:t6: OK. It was a wild idea