Open
Conversation
Hi! Thanks for all the work on this project and with `gazelle-origin`. These projects are looking to be a lifesaver for my music collection.
I've opened this PR because this change solves an issue I've been facing with `albumdisambig`. Further, I believe the new behavior is closer to the desired behavior. Basically, I'd like for the `originquery` plugin to add the `albumdisambig` tag to the files when it's listed in `originquery.tag_patterns`. Currently, `originquery` will only add that tag when it's included in the `musicbrainz.extra_tags` config block. If `albumdisambig` is added to that block, there can be unintended downstream effects on the `musicbrainz` plugin (and generally it's not desired that `originquery` configuration is "hidden" in `musicbrainz` configuration). For example, the import breaks if `origin.yaml` doesn't have the `"Edition"`:
```
beets-beets-1 | File "/usr/local/lib/python3.8/site-packages/beets/autotag/mb.py", line 498, in match_album
beets-beets-1 | key = FIELDS_TO_MB_KEYS[tag]
beets-beets-1 | KeyError: 'albumdisambig'
```
To illustrate, here is my case study of the current behavior:
----------------------------
I'm attempting to import two versions of Adele's 30. The `origin.yaml` files provide the correct "Edition" field across both releases. For the original, `"Edition"` is blank. For Target exclusive, `origin.yaml` contains `"Edition": Target Exclusive`. So far so good. However, if I run a `beet import` with this config:
```yaml
musicbrainz:
extra_tags:
- year
- catalognum,
- country,
- label,
- media
originquery:
use_origin_on_conflict: yes
origin_file: origin.yaml
tag_patterns:
media: '$.Media'
year: '$."Edition year"'
label: '$."Record label"'
catalognum: '$."Catalog number"'
albumdisambig: '$.Edition'
```
Beets doesn't add the `albumdisambig` tag during autotagging. It's skipped over because `albumdisambig` isn't in `musicbrainz.extra_tags`. As such, the albums are missing the `albumdisambig` entirely and the files end up in the filesystem like:
```
30 (<backup_disambiguator>)
30 (<backup_disambiguator>)
```
But what I want is:
```
30 (<backup_disambiguator>)
30 (Target Exclusive)
```
-------------------------------------------------
I'd like the `"Edition"` field to populate those parentheses (through `albumdisambig`). The change in this PR fixes this issue by attempting to tag the files if it's in the `originquery.tag_patterns` config block. This also opens up the possibility for additional tags based on `origin.yaml`, if that's desired.
I've testing this change against a few dozen albums and everything works as I'd expect.
Author
|
@x1ppy Have you gotten a chance to take a look at this? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hi! Thanks for all the work on this project and with
gazelle-origin. These projects are looking to be a lifesaver for my music collection.I've opened this PR because this change solves an issue I've been facing with
albumdisambig. Further, I believe the new behavior is closer to the desired behavior. Basically, I'd like for theoriginqueryplugin to add thealbumdisambigtag to the files when it's listed inoriginquery.tag_patterns. Currently,originquerywill only add that tag when it's included in themusicbrainz.extra_tagsconfig block. Ifalbumdisambigis added to that block, there can be unintended downstream effects on themusicbrainzplugin (and generally it's not desired thatoriginqueryconfiguration is "hidden" inmusicbrainzconfiguration). For example, the import breaks iforigin.yamldoesn't have the"Edition":To illustrate, here is my case study of the current behavior:
I'm attempting to import two versions of Adele's 30. The
origin.yamlfiles provide the correct "Edition" field across both releases. For the original,"Edition"is blank. For Target exclusive,origin.yamlcontains"Edition": Target Exclusive. So far so good. However, if I run abeet importwith this config:Beets doesn't add the
albumdisambigtag during autotagging. It's skipped over becausealbumdisambigisn't inmusicbrainz.extra_tags. As such, the albums are missing thealbumdisambigentirely and the files end up in the filesystem like:But what I want is:
I'd like the
"Edition"field to populate those parentheses (throughalbumdisambig). The change in this PR fixes this issue by attempting to tag the files if it's in theoriginquery.tag_patternsconfig block. This also opens up the possibility for additional tags based onorigin.yaml, if that's desired.I've testing this change against a few dozen albums and everything works as I'd expect.