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 type annotations for Spotify JSON objects #695

Draft
wants to merge 12 commits into
base: v3
Choose a base branch
from

Conversation

akathorn
Copy link

Working on #439. It is WIP but I would like some feedback before I validate the return types one by one.

All public methods are annotated. I haven't added anything that alters the runtime behavior in any way.
The arguments are straightforward since they are all basic types, but I had to define new types for the return values. The alternative is using Dict[str, Any] for all the JSON dicts, which would be more "lightweight" but provides near-zero typing information.

It works on nearly examples!

"expires_in": int,
"refresh_token": str,
"expires_at": int
})
Copy link
Contributor

Choose a reason for hiding this comment

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

scope and refresh_token will not be present for the client credentials flow, and so they should be optional.

Copy link
Member

Choose a reason for hiding this comment

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

In the future we could look into creating a new type if possible, but probably not a worry now

Copy link
Author

Choose a reason for hiding this comment

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

I discussed this here. We can either:

  • Ignore the fact that they will not be present
  • Create a class TokenInfo
  • Remove this, use Dict[str, Any], and wait until TypeDict supports optional keys (or just forget it)

"total": int,
})

PrivateUser = TypedDict("PrivateUser", {
Copy link
Contributor

Choose a reason for hiding this comment

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

The email, explicit_content, product, and country properties of the PrivateUser object can still be nil if certain scopes are missing, and so these should also be optional. See here.

Copy link
Author

Choose a reason for hiding this comment

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

There is the same issue that I described here.

@stephanebruckert stephanebruckert linked an issue Jun 26, 2021 that may be closed by this pull request
@akathorn akathorn changed the title Add type annotations Add type annotations for Spotify JSON objects Jul 1, 2021
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.

Support for type hints
3 participants