-
Notifications
You must be signed in to change notification settings - Fork 214
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
Type validation with pydantic #307
Comments
Pydantic v2 ETA is now Q1 2023, so we can start work on this Q2 at the earliest |
I am willing to help :) |
Pydantic v2 is now in alpha: https://docs.pydantic.dev/blog/pydantic-v2-alpha/ I think we should use this issue as a starting point to create additional subtasks using GitHub tasklists: https://docs.github.com/en/issues/tracking-your-work-with-issues/about-tasklists As this is a major endeavor with breaking changes development should take place on a v2 branch for now. |
Started playing around with this, here is a rudimentary way to provide type validation. I created a new module called where I started to define some types. The
Then, inside "parsers/browsing.py", we can create a new instance of this
(Note, this could probably be done at a "lower level" without having to create the intermediary Then, we can add some type hinting in "mixins/library.py"...
And now we get some more helpful type hinting with the new object. The data validation works too. For example, in the
I can continue to help with this work - I think it can be very helpful for maintaining the API as well as improving usability with the type hints. Here's a quick branch comparison between the testing I did and the current master branch: https://github.com/sigma67/ytmusicapi/compare/master...JohnHKoh:ytmusicapi:v2-pydantic-test?expand=1 |
Looks like a good start! I like that it's not very invasive to the current code base, at least not in this case. I think we should probably start by dumping the currently returned responses to make sure we don't miss any fields. Then we can have a look at the code to determine which fields need to be optional, for each function. Did you use pydantic V2 beta? I think you'd also need to add that to the dependencies. I also noticed that you used |
Definitely, anywhere we can consolidate these? Should we just post them in here? The greater the sample size the better, so we don't miss any edge cases or differences.
I was using v2 beta in a venv, but yes, it will need to be added. I was just playing around with integrating it into the code, so nothing I wrote was really meant to be final! Just a start. The way we "convert" the raw data into the new types is definitely an implementation detail that could warrant more discussion. Should it use some kind of constructor? Or some other generic parsing function?
I also support the push |
I don't think we need to many different samples, just need to ensure that the edge cases are covered (like unavailable songs, which have some data fields, but not others). Instead of dumping we could also just go ahead and try with all fields optional and default None. That would already be an improvement over the current case where sometimes the keys are missing, making the library a bit cumbersome to use.
Definitely just use the default constructor
Then let's go ahead with |
Hi, I recently made an API Wrapper that used Pydantic. https://github.com/TheOnlyWayUp/Wattpad-Py A tool I used prominently was https://jsontopydantic.com. I'm going to draft up a PR this week. |
Something I've been meaning to implement in the long term is proper models and type validation for the responses from YouTube Music. This would provide better data validation and would notify sooner if something on the API side has changed, as opposed to the current tests which usually check for the length of the results in the response only.
pydantic seems to be the gold standard for this. I'd like to wait for the release of v2.0 to not have to deal with breaking changes from v1 once it comes out.
Please thumbs up if you're in favor and leave a comment if you're willing to help or contribute 🥳
The text was updated successfully, but these errors were encountered: