-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Add aliases support #5277
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Add aliases support #5277
Conversation
I would really like this functionality. However, there is #4936, which completely replaces the I would ask you to consider looking at that PR and how you can use it, since I will begin reviewing it now. |
Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward? This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
The replacement is stale for quite a while now. And because of the size of the change and impact, it requires a lot of developers and beets maintainers availability. Pragmatically, It can not be expected to happen soon.
I tend to agree with that statement, this and maybe other propositions may continue to improve beets in the meantime without being blocked by uncertain deep refactoring. As long as new propositions are tested, it should mitigate the risk of regression that the new musicbrain binding change could be subjected to. Could it be reconsidered ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This branch needs to be rebased on master since there's been a fair bit of updates since the PR was opened.
Additionally, it would be great if you could adjust one of the tests to show that this new title resolution works as intended.
``medium_index``, the track's index on its medium; ``medium_total``, | ||
the number of tracks on the medium. Each number is a 1-based index. | ||
""" | ||
if "alias-list" in recording: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is duplicated in 4 places. Could you define a function to deduplicate it?
I don't know if @sezaru plans to resume his work else I can continue it. But before, there is this requirement that needs to be sorted:
Would the beets maintainers agree to fork it to accept MR to extend beets features ? |
@Morikko, this may be slightly complicated. We won't be able to publish a new version of the package and will have to depend on the GitHub URL of the fork, which is not ideal. MusicBrainz have a simple JSON API, I wonder whether we could remove this dependency and talk to MB directly. Let me see how much effort would this require. |
Here we go: #6052 |
Description
This PR add support for aliases to releases, release-groups and recordings.
This PR is a must have (IMO at least) for people that listen to Japanese, Chinese and other songs that has other symbols for letters. With this, not only the artist name will use the alias if available, but now the album and track name will also use aliases.
NOTE: This PR requires a fork of the
python-musicbrainzngs
package. That package seems to be dead (more than 2 years without any commit) and is lacking in a lot of updates to Musicbrainz API.Here is my PR adding support for the new fields alastair/python-musicbrainzngs#289 but I have no hope that will be merged since the package seems to be dead.
Personally I think beets can create a fork of that package and just add the bare minimum to it to make new features work (for example this one).
I didn't add any tests and changelog changes yet since this PR requires forking
python-musicbrainzngs
, so first I want to discuss if this can be merged and then after getting the confirmation I will add the missing parts.