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

Use regular expression for new_div's class name #154

Merged
merged 3 commits into from
Sep 20, 2020

Conversation

eeishaan
Copy link
Contributor

I observe that many times the class name of a new_div is not the same for me as hard-coded in the file. Using a regular expression instead of a fixed string to search for the lyrics' div solves the problem.

I have attached the html source that I get for my query song = genius.search_song('Would You Be So Kind', 'dodie'). Please change the extension to .html. I've uploaded as .txt because github doesn't allow me to upload .html files: source.txt

Use a regular expression instead of a fixed string to  search for the lyrics' div
@eeishaan
Copy link
Contributor Author

Woops! Just saw the merge request #153 that handles this efficiently. Closing this.

@eeishaan eeishaan closed this Aug 23, 2020
@allerter
Copy link
Collaborator

allerter commented Aug 23, 2020

I didn't know you could send a Regex as the class_ parameter. So with that in mind, I think with a small modification:

new_div = html.find("div", class_=re.compile("Lyrics__Root"))

Not only is it more concise but also more efficient (checked with timeit) than the one in #153. So if you could please make that modification, I think this is the way to go to get the div.

Finding the lyrics div in one go was a good idea, but `div.get_text('\n')` didn't work well when the old div was used. So this one is better.
@johnwmillr johnwmillr merged commit 6304fdb into johnwmillr:master Sep 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants