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

get_album: track_number returns none when track unavailable + test #516

Merged
merged 3 commits into from
Jan 14, 2024

Conversation

jcbirdwell
Copy link
Contributor

Simple fix for #515 and adds test case for albums with unavailable tracks.

Issue could alternatively be solved by extending nav's none_if_absent to coerce nullish return values. thoughts?

tests/conftest.py Outdated Show resolved Hide resolved
ytmusicapi/parsers/playlists.py Outdated Show resolved Hide resolved
@sigma67 sigma67 linked an issue Jan 13, 2024 that may be closed by this pull request
@jcbirdwell
Copy link
Contributor Author

Side note on the new pipeline, these code coverage checks that keep failing. Am I supposed to be doing something differently? My local tests are passing.

Also I've yet to get mypy to run correctly, it always throws errors about missing type stubs related to requests, despite having installed them via pip and mypy directly. Any recommendations?

ytmusicapi/auth/oauth/credentials.py:5: error: Library stubs not installed for "requests"  [import-untyped]
ytmusicapi/auth/oauth/token.py:8: error: Library stubs not installed for "requests.structures"  [import-untyped]
ytmusicapi/mixins/_protocol.py:4: error: Library stubs not installed for "requests"  [import-untyped]
ytmusicapi/auth/browser.py:5: error: Library stubs not installed for "requests.structures"  [import-untyped]
ytmusicapi/mixins/uploads.py:5: error: Library stubs not installed for "requests"  [import-untyped]
ytmusicapi/setup.py:6: error: Library stubs not installed for "requests"  [import-untyped]
ytmusicapi/setup.py:6: note: Hint: "python3 -m pip install types-requests"
ytmusicapi/setup.py:6: note: (or run "mypy --install-types" to install all missing stub packages)
ytmusicapi/setup.py:6: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports
ytmusicapi/ytmusic.py:10: error: Library stubs not installed for "requests"  [import-untyped]
ytmusicapi/ytmusic.py:12: error: Library stubs not installed for "requests.structures"  [import-untyped]

@sigma67
Copy link
Owner

sigma67 commented Jan 14, 2024

Don't worry about the test/coverage check, it cannot work yet for external contributors. Right now it only works for people with access to the secrets of this repo, because that's how GitHub does Pull Requests. To get it to work you'd have to set the following variables in your fork (just copy paste the text of the equivalent files you use locally):

image

Normally you'd only have to run mypy --install-types once, then run it again. Are you running through precommit?

@sigma67 sigma67 merged commit 2b42c58 into sigma67:main Jan 14, 2024
3 of 4 checks passed
@jcbirdwell
Copy link
Contributor Author

To get it to work you'd have to set the following variables in your fork (just copy paste the text of the equivalent files you use locally):

Alright cool. I'll give that a whirl.

Normally you'd only have to run mypy --install-types once, then run it again. Are you running through precommit?

Yea I've run mypy --install-types and tried installing the stub directly with pip, still throws. Yea I'm running via precommit.
I ran across something similar as an open issue on the mypy git python/mypy#16400 and was just wondering if you had something setup differently on your machine to avoid it. Outside of trying to install the stubs I setup/configured the environment with pdm/pipx as per the contributing.rst so I'm unsure where the difference would be.

@sigma67
Copy link
Owner

sigma67 commented Jan 14, 2024

Seems to be a common issue with the precommit mypy action, I remember having some trouble with it as well. Try this: https://stackoverflow.com/a/73603491/5726546

Feel free to update the file in a PR if it works for you

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.

track_number fails on disabled tracks
2 participants