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

Fix error when album contains only year information #182

Merged
merged 2 commits into from
Apr 17, 2021
Merged

Fix error when album contains only year information #182

merged 2 commits into from
Apr 17, 2021

Conversation

gal20
Copy link
Contributor

@gal20 gal20 commented Feb 13, 2021

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
Copy link
Collaborator

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

@allerter
Copy link
Collaborator

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
Copy link
Contributor Author

gal20 commented Feb 13, 2021

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
Copy link
Collaborator

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.

Copy link
Owner

@johnwmillr johnwmillr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR, @gal20! I'll merge your changes now.

@johnwmillr johnwmillr merged commit fa95285 into johnwmillr:master Apr 17, 2021
@allerter allerter added this to the 3.0.1 milestone Apr 21, 2021
@gal20 gal20 deleted the datetime_fix branch May 29, 2021 12:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants