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 parameter type hinting to API reference #895

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

kevinsekuj
Copy link
Contributor

@kevinsekuj kevinsekuj commented Nov 26, 2022

  • Added basic built-in type hints to client.py methods
  • Modified docstrings of all methods with parameters from the form
            Parameters:
                - result - a previously returned paged result

to

:param result: a previously returned paged result

Example:

image

@stephanebruckert
Copy link
Member

Indeed type hinting is something we will need at some point, however since spotipy 2.x still supports python 2.7, I wonder if type hinting isn't going to break on 2.7? If it is breaking it that's fine, we can still add type hinting in spotipy 3.x, see #652. In that case the PR must be raised against the v3 branch. However please be aware there is already #701 and #695

@kevinsekuj
Copy link
Contributor Author

kevinsekuj commented Nov 26, 2022

I see, thanks for the info. I was aiming more towards adding typing to the API reference documentation, using this package for sphinx, which auto generates the typing in the docs based on the source code.

So when adding type hints to function parameters and setting up the docstrings correctly, the docs build looks something like

image

I see that #701 uses more thorough types from the typing package, fortunately sphinx-autodoc-typehints should be compatible with those too.

Let me know your thoughts -- I can leave it up as well if it might be useful to someone later on since manually editing all the docstrings is time consuming

@kevinsekuj kevinsekuj marked this pull request as ready for review November 26, 2022 23:55
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.

2 participants