johnwmillr / LyricsGenius

Download song lyrics and metadata from Genius.com 🎶🎤
http://www.johnwmillr.com/scraping-genius-lyrics/
MIT License
892 stars 158 forks source link

Fix error when album contains only year information #182

Closed gal20 closed 3 years ago

gal20 commented 3 years ago

Tested with Hadestown (2010). On master, I get the following exception:

>>> album = genius.search_album("Hadestown", "Anaïs Mitchell")
Searching for "Hadestown" by Anaïs Mitchell...
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/gal/.local/lib/python3.9/site-packages/lyricsgenius/genius.py", line 356, in search_album
    return Album(self, album_info, tracks)
  File "/home/gal/.local/lib/python3.9/site-packages/lyricsgenius/types/album.py", line 17, in __init__
    self.release_date_components = convert_to_datetime(
  File "/home/gal/.local/lib/python3.9/site-packages/lyricsgenius/utils.py", line 57, in convert_to_datetime
    if f.count('-') == 2:
AttributeError: 'int' object has no attribute 'count'

The error is caused by this line: date = int(year) when date should be a string.

I've fixed this issue and simplified the function a bit.

allerter commented 3 years ago

Hi. Thanks for your PR. I think it's good to go.

allerter commented 3 years ago

Looking at the Album attributes got me thinking. The album dict already has a release_date key alongside the release_date_components. I don't know why I chose to use release_date_components instead because release_date is just what we need for converting the release date to a datetime type. We should've used release_date for three reasons:

  1. release_date_components has the accurate date; if the release date has no month or day, its value will just be None. But that's not the case for release_date. The release_date value will display what it has and replaces what it doesn't with 01, just how Python does in datetime objects!
  2. We should convert release_date to datetime to offer a convenient way for accessing the date. And we should keep release_date_compnents as it is for users who want an accurate date.
  3. Converting release_date to datetime is easy; it's either None or a string with the %Y-%m-%d format.
gal20 commented 3 years ago

I'm not familiar with the internals of this module, I simply got this exception and applied a quick fix. Feel free to close this PR if you intend to drastically alter or remove this function.

allerter commented 3 years ago

Don't mind that comment. It's just meant for the developers. Your PR is good to go and it'll probably be merged soon.