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

Add tempo information to track metadata #368

Merged
merged 8 commits into from
Jan 1, 2024
Merged

Add tempo information to track metadata #368

merged 8 commits into from
Jan 1, 2024

Conversation

krishsharma0413
Copy link
Contributor

fixes #367

@krishsharma0413
Copy link
Contributor Author

All 12 tests are passing but I don't have any idea on how to reflect that here.
Please guide me with these checks.

.cache Outdated Show resolved Hide resolved
@SathyaBhat
Copy link
Owner

All 12 tests are passing but I don't have any idea on how to reflect that here. Please guide me with these checks.

I don't see any changes to the tests - are they committed?

@krishsharma0413
Copy link
Contributor Author

I was waiting for your approval for creation of new tests for the BPM itself. I will create them ASAP

@krishsharma0413
Copy link
Contributor Author

hey @SathyaBhat,
Does this work? Please tell me if there needs to be anymore changes.

@SathyaBhat
Copy link
Owner

hey @SathyaBhat, Does this work? Please tell me if there needs to be anymore changes.

the tests seem to be still failing https://github.com/SathyaBhat/spotify-dl/actions/runs/7364433669/job/20044825544?pr=368

                    track_album = track["album"]
>                   track_tempo = track["tempo"]
E                   KeyError: 'tempo'


@krishsharma0413
Copy link
Contributor Author

oh it's because i didn't update those test cases. let me fix those as well

@krishsharma0413
Copy link
Contributor Author

The code checks for the BPM in spotify.py and since the test cases are made custom rather than going through the library they don't have the tempo as a key-value pair. I can fetch the data of the music manually and assign the tempo to it.

Copy link
Owner

@SathyaBhat SathyaBhat left a comment

Choose a reason for hiding this comment

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

some minor suggestions else looks good to me. thanks for your contribution!

tests/test_spotify_fetch_tracks.py Outdated Show resolved Hide resolved
spotify_dl/youtube.py Outdated Show resolved Hide resolved
@SathyaBhat SathyaBhat merged commit a0d4bcc into SathyaBhat:master Jan 1, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add BPM to metadata of MP3 songs.
2 participants