-
Notifications
You must be signed in to change notification settings - Fork 958
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
Support for type hints #439
Comments
Hey @roeekl, looks like this is new in python3.5. If there is a way to add it while keeping compatibility with older python versions, we would definitely merge such PR! |
@stephanebruckert What is the minimum version of Python that spotipy 3 will need to support (considering 3.5 is already past EoL and the 3.6 EoL will probably pass before spotipy 3 comes out)? 3.7 introduced |
@IdmFoundInHim I think we should choose what's easy for us. Developers should always think of upgrading their own version of python3. Starting at 3.7 sounds good to me? |
I use pylance strict type-checking in my project and the lack of type hints in Spotipy was irking me, so I started playing around with my own .pyi stubs using stubgen as a starting point. I have a small example of the type-checking here. My favorite thing about this approach is that vscode (and certainly other tools) autocompletes the fields of the response: Point is: if there is interest in this approach, I would be willing to go further with it and contribute it :) |
@akathorn Open a pull request on the |
I ran into an issue when representing Spotify JSON objects using Since there is more complexity in representing Spotify JSON objects, I'm going to split this work in two and send a simpler pull request annotating just the function parameters. I'll leave the return types as The current pull request will be for the more complex work of representing Spotify objects in the type system. Regarding Spotify objectsAs I mentioned, the problem right now is that keys in a
The type checker will report that the type of I'll think about it, but at this point I see two options:
|
Actually, there is a We could also take a completely different approach to the spotify objects: We could decode them into data classes instead of using dictionaries. tekore has already done this. I admit I like this idea more. |
Implementing dataclasses is a much bigger task than I would rather us not implement data from spotipy in a way that forces other projects to use an abundance of classes. I think we implement as |
I thought about if "name" in album:
name = album["name"] Or alternatively I personally think this makes using On the other hand, with restriction = album["restrictions"] # Type: Any
print("things are restricted! Because: " + restriction["reason"]) # Type-checks This is basically saying "I don't have a clue about what is inside", while using My point is that I would prefer a solution that gives very little information to one that provides somewhat-precise information, until |
How about we use the workaround mentioned in the PEP's Motivation section? That is, use two TypedDict, one with the required keys and another with the not-required keys, liked through inheritance. When the ideal solution is supported, it should be relatively easy to programatically upgrade to the new syntax. |
*facepalm* 🤦🏻♂️ I totally missed this. This will certainly work, I'm gonna give it a go :) |
By the way, I've created a Spotify web API library in Swift which has a struct for each object in the Spotify object model. Swift requires you to explicitly mark which properties might be If you're not familiar with swift, here's a quick guide on how to read the syntax: public struct Artist: Hashable {
let name: String
// The '?' indicates this could be `None`
let uri: String?
let id: String?
// Either an array of `SpotifyImage`, or `None`
let images: [SpotifyImage]?
let popularity: Int?
// Either a dictionary of with keys of type `String` and values
// of type `URL`, or `None`.
let externalURLs: [String: URL]?
let followers: Followers?
let genres: [String]?
let href: URL?
let type: IDCategory
} |
Thank you, this is going to be useful. Am I understanding it correctly, that the JSON decoder uses Any suggestions on how to figure out what could be missing and what could be null? The only thing I can think of is checking out other libraries out there, but I imagine it could be quite common that they abstract missing keys and null values in a similar way. Tekore seems to be doing it too, as far as I can see. |
Yes, that is correct. However, I will look into ways of distinguishing between these cases.
Yes, it's extremely frustrating. Apparently, the developers were too lazy to document each property that might be |
Can I take a look at those tests? I wrote some code to automatize comparing a response with the expected |
The tests are here, in the same repository. However, they are unlikely to be useful to you as they are also written in Swift. Essentially, they involve making calls to each of the API endpoints and ensuring the response is successfully decoded into the expected type. They aren't going to help you distinguish between |
I'm hoping for good examples of calls that return missing keys or def test_track_urn(self):
track = self.spotify.track(self.creep_urn)
self.assertType(track, self.spotify.track)
Which shows that the key My idea is finding a lot of examples, trying to be as exhaustive as possible, and correct the types until they fit every known case. There would certainly be a lot of false negatives because of edge cases and outliers for which there are no tests. But if we can't find an alternative, maybe fitting most cases would be good enough? |
Writing extensive unit tests is the only reliable way to validate the type hints. That's what I did for my library. Here's one tip: Make sure you run tests with local tracks. These tracks contain lots of missing data. They may not even have URIs! You have to add them to a playlist and then make a request for that playlist. Here's one of my playlists with local tracks: |
Having typed objects for each possible output Spotify API is able to return would greatly improve development workflow in IDEs. |
I didn't wrap this up in the end, in great part due to a chronic illness. |
It would be nice to add a support for type hints. I know most of the functions have
str
parameters and returningDict
but it still can help in other functions likeprompt_for_user_token
.The text was updated successfully, but these errors were encountered: