reconfigured types and added album, added retries and etc#162
Conversation
moved Artist and Song to types added TokenRequiredError updated docs,
johnwmillr
left a comment
johnwmillr
left a comment
There was a problem hiding this comment.
@allerter, thank you for these changes!
How many methods do we have overlapping between the developer API and the public, token-less API? Whenever there is overlap, I would prefer that we fall back to the developer API and require a token. I want to encourage as much authenticated use of the Genius API as possible.
added public_api to methods (incomplete)
added the public_api attr to Genius, removed pytest from dependencies
update TokenRequiredError, update overlapping methods
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This reverts commit 3386282644198706639e858098af18b560976ccd.
updated search_song, updated genius.py imports
|
|
||
| def tearDown(): | ||
| @classmethod | ||
| def tearDownClass(cls): |
There was a problem hiding this comment.
Since TestUtils, is the last test to run, this method will close the session and we no longer will get the unclosed-socket warnings. But there's still a problem.
I ran the tests for 2.0.0 and in the end, only got one warning. But at the end of the tests for the current version, there are two warnings. This might mean that we are opening two sockets which is unnecessary. At the same time, genius._session.close() makes both of the warnings go away which is curious.
fixed a ArtistMethods test, updated genius.song_annotations and genius.lyrics
add docs
add examples to snippets
update docs
added artist to Song.to_dict
- added Song.lyrics defaulting to "" if lyrics amounts to False (when None is passed) - added printing a more descriptive message when lyrics section isn't found, - fixed a snippet
- added "discussions" public API endpoint, - updated docs
|
Hi, @johnwmillr. Please review this when you get a chance. |
|
Hi, @allerter. It's been a busy few weeks for me, but I do plan to review this PR. Thanks for your work on it! I'll get to work reviewing it this week. |
reconfigured Genius.lyrics() to:
* fetch the page through the Sender
* extract the lyrics not only more concisely, but also without changing the original format of the lyrics (adding newlines)
* reconfigured Sender to:
* allow making requests to Genius at https://genius.com
* remove SLEEP_TIME to allow users to supply their own rate limiting (by default 0.2) instead of enforcing it
* move the authorization header to the authorization_header attribute instead of re-creating it for every API call
* to check lyrics_state along with the title
* reconfigured search_* that return an object:
* to check lyrics_state to avoid getting empty lyrics
* moved setting excluded_terms to __init__
* moved _clean_str to utils and renamed to clean_str
* added tests for genius.tag
* reordered some of the imports and the functions alphabetically
- fix genius.tag() example
johnwmillr
left a comment
johnwmillr
left a comment
There was a problem hiding this comment.
Thanks for your work on this extensive PR, @allerter. I made a couple fixes to typos and pointed out two tests that are failing in my local environment. Once we sort that out it should be good to go.
* added text_format parameter to account() * added try/finally close to test_manage_annotation to try to delete created annotation if an exception occurs
johnwmillr
left a comment
johnwmillr
left a comment
There was a problem hiding this comment.
All tests are passing for me. Thanks, @allerter!
2 participants
Geniu.retriesfor Timeout and HTTPErrors with a >= 500 status code.