MiataBoy / exorium

esquire, a multifunctional bot to fit your needs.
https://discord.gg/XUS6jcD7n7
GNU Affero General Public License v3.0
0 stars 0 forks source link

Revert "Update Lyrics to use SRA not LyricsFinder" #113

Closed Aresiel closed 3 years ago

Aresiel commented 3 years ago

This reverts commit 253b13d5e5dfe1a6327fa54bccdab677e6da085a, the commit Duck made that switched over from lyricsgenius to SRA. We cannot use SRA because SRA yields seemingly random inconsistent responses.

Details The algorithm used to paginate the lyrics splits the lyrics by verses. It does this in order to provide a nice reading experience to the user. Verses are identified by the double-newline "\n\n" formation separating them. SRA sometimes returns different song lyrics not featuring these double-newlines. These poorly-formatted lyrics cannot be split using our algorithm, and even unsplit, provide a terrible reading experience. Example below.

API Request: https://some-random-api.ml/lyrics?title=pi%20song Reponse 1:

And now
AsapSCIENCE presents
100 digits of pi

3.14159, this is pi
Followed by 2-6-5-3-5-8-9
Circumference over diameter

7-9, then 3-2-3
OMG! Can't you see?
8-4-6-2-6-4-3
And now we're on a spree

38 and 32, now we're blue
Oh, who knew?
7,950 and then a two
88 and 41, so much fun
Now a run
9-7-1-6-9-3-9-9
Then 3-7, 51

Halfway done!
0-5-8, now don't be late
2-0-9, where's the wine?
7-4, it's on the floor
Then 9-4-4-5-9

2-3-0, we gotta go
7-8, we can't wait
1-6-4-0-6-2-8
We're almost near the end, keep going

62, we're getting through
0-8-9, 9 on time
8-6-2-8-0-3-4
There's only a few more!

8-2, then 5-3
42, 11, 7-0 and 67
We're done! Was that fun?
Learning random digits
So that you can brag to your friends

Response 2:

And now
AsapSCIENCE presents
100 digits of pi3.14159, this is pi
Followed by 2-6-5-3-5-8-9
Circumference over diameter7-9, then 3-2-3
OMG! Can't you see?8-4-6-2-6-4-3
And now we're on a spree38 and 32, now we're blue
Oh, who knew?7,950 and then a two88 and 41, so much fun
Now a run9-7-1-6-9-3-9-9
Then 3-7, 51
Halfway done!0-5-8, now don't be late2-0-9, where's the wine?7-4, it's on the floor
Then 9-4-4-5-92-3-0, we gotta go7-8, we can't wait1-6-4-0-6-2-8
We're almost near the end, keep going62, we're getting through0-8-9, 9 on time8-6-2-8-0-3-4
There's only a few more!8-2, then 5-342, 11, 7-0 and 67
We're done! Was that fun?
Learning random digits
So that you can brag to your friends 

Where instead of newlines, there is no whitespace at all, merely concatenating the different lines together. Both responses vary with no discernable pattern. If you wish to verify this, simply go to https://some-random-api.ml/lyrics?title=pi%20song and refresh a few times.

Aresiel commented 3 years ago

NOTE: The command will error unless the variable GENIUSTOKEN in config.py is set to a valid genius.com token.

Aphidian commented 3 years ago

I agree with this.

SRA is incredibly inconsistent.

DuckMasterAl commented 3 years ago

The issue with lyricsgenius is it uses requests, which is not asynchronous, to make its API calls. You can read about why this is an issue here: https://discordpy.readthedocs.io/en/stable/faq.html#what-does-blocking-mean The library blocks the heartbeat connection to Discord, and it could cause latency issues. You can solve this issue by directly calling to the API using aiohttp.

I'm going to remove my review on this as this is not a critical issue, but it's a recommendation since it could cause issues with the bot in the future.

Aresiel commented 3 years ago

The issue with lyricsgenius is it uses requests, which is not asynchronous, to make its API calls. You can read about why this is an issue here: https://discordpy.readthedocs.io/en/stable/faq.html#what-does-blocking-mean The library blocks the heartbeat connection to Discord, and it could cause latency issues. You can solve this issue by directly calling to the API using aiohttp.

I'm going to remove my review on this as this is not a critical issue, but it's a recommendation since it could cause issues with the bot in the future.

Commands are asynchronous functions, do asynchronous functions not run in their own threads?

Aphidian commented 3 years ago

The issue with lyricsgenius is it uses requests, which is not asynchronous, to make its API calls. You can read about why this is an issue here: https://discordpy.readthedocs.io/en/stable/faq.html#what-does-blocking-mean The library blocks the heartbeat connection to Discord, and it could cause latency issues. You can solve this issue by directly calling to the API using aiohttp.

I'm going to remove my review on this as this is not a critical issue, but it's a recommendation since it could cause issues with the bot in the future.

What if I fork the module and edit it to use aiohttp?

Using SRA is very difficult and it would be a pain to work with.

Aresiel commented 3 years ago

The issue with lyricsgenius is it uses requests, which is not asynchronous, to make its API calls. You can read about why this is an issue here: https://discordpy.readthedocs.io/en/stable/faq.html#what-does-blocking-mean The library blocks the heartbeat connection to Discord, and it could cause latency issues. You can solve this issue by directly calling to the API using aiohttp. I'm going to remove my review on this as this is not a critical issue, but it's a recommendation since it could cause issues with the bot in the future.

What if I fork the module and edit it to use aiohttp?

Using SRA is very difficult and it would be a pain to work with.

That would solve the problem

Aphidian commented 3 years ago

I have made an async version of lyricsgenius: https://github.com/soulrika/LyricsGenius

Just replace genius.search_song(title) with await genius.search_song(title)