Skip to content
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

[FEATURE] multiple lyrics sources #2682

Open
martinmake opened this issue Nov 20, 2020 · 2 comments
Open

[FEATURE] multiple lyrics sources #2682

martinmake opened this issue Nov 20, 2020 · 2 comments
Labels
enhancement New feature or request

Comments

@martinmake
Copy link

Many songs that I like to listen to aren't avaible on Genius, but they are avaible on Musixmatch or Tekstowo.pl.

I would like you to add an ordered list of lyrics sources with more than just Genius by default (at least Musixmatch).

@martinmake martinmake added the enhancement New feature or request label Nov 20, 2020
@aadibajpai
Copy link
Member

hey! thanks for the issue.

yeah, I agree with you that genius lyrics database is somewhat lacking when it comes to non english songs.

however, using genius and not multiple providers is a deliberate design choice—SwagLyrics is able to be this fast because we don't search for the song and are opinionated. Using MusixMatch might increase availability for a few users but would decrease the overall response times for the requests it falls back on MusixMatch for. As for not using MusixMatch as the primary source of lyrics—Genius quality is simply way better than MusixMatch.

Which is why I'm not sure if this is a direction we will be going into.

With that said, I noticed you started working on the integration already at #2683, what I would encourage you to do is to use your fork instead and if it works well I'd love to list it in the README or something for users who would also prefer to have the MusixMatch option.

@martinmake
Copy link
Author

Well, I would argue, that a response is better than an error message (no response), when it comes to responsiveness. I've only implemented it as a fallback, so lyrics that are available on Genius should stay really fast.

So I don't really understand why we shouldn't merge. Could you perhaps explain, where this loss of responsiveness comes from? But sure, if we wont find a common ground we could definitely just keep it as a fork and add it to README.

I might open another issue soon. It will be for fuzzy searching on Genius as a fallback to precise url generation from information from Spotify, because I've had some issues with lyrics being avaible on Genius, but only under non-standard url, this is not a problem with how I query Musixmatch (It's slower overall, but failing the fast lookup and then having to go through a slower fallback could end up feeling worse).

For Genius, there is another cool opt-in way we could get better lyrics queries and that's through their API. I assume it would also boost responsiveness. If you want, I could open an issue for it and also implement it in an opt-in non-disruptive way.

My implementation even finds and correctly outputs lyrics for songs with artist and song name being non-latin and thus it breaks one of your tests:

=================================== FAILURES ===================================
_______________________ Tests.test_that_get_lyrics_works _______________________

self = <tests.integration.Tests testMethod=test_that_get_lyrics_works>

    def test_that_get_lyrics_works(self):
        """
        Test that get_lyrics function works
        """
>       self.assertEqual(get_lyrics('果てるまで', 'ハゼ馳せる'), None)  # song and artist non-latin
E       AssertionError: '沈むように溶けてゆくように\n二人だけの空が広がる夜に\n\n「さよなら」だけだ[809 chars]していく' != None

tests/integration.py:23: AssertionError

I had no problems displaying non-latin characters in my terminal emulator and I'm sure other users can either figure out how to get them to display correctly, or they can just ignore them rather than having displayed an error message for all users.
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants