-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Properly type db on the model, sort out generic types issues #5545
Conversation
Thank you for the PR! The changelog has not been updated, so here is a friendly reminder to check if you need to add an entry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Woohoo, this really looks like the pieces are falling into place for wrapping up typings of beets' core 🎉
I left a few small nitpicks, overall this is looking great!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! Feel free to merge this after addressing the question of where the SmartArtistSort
should go.
Thanks to @wisp3rwind's suggestion this PR adds types to the relationship between `Model`, `Database` and `Library`. Then I worked through the rest of the issues found in the edited files. Most of this involved providing type parameters for generic types (or defining defaults, rather 😉). There `queryparse` module had a somewhat significant issue where the sorting construction logic only expected to receive `FieldSort` subclasses, while `SmartArtistSort` was not one. Thus `SmartArtistSort` has now been forced to behave and is a `FieldSort` subclass. It's also been moved to `query.py` module which is where the rest of sorts are defined.
Thanks to @wisp3rwind's suggestion this PR adds types to the relationship between
Model
,Database
andLibrary
.Then I worked through the rest of the issues found in the edited files. Most of this involved providing type parameters for generic types (or defining defaults, rather 😉).
There
queryparse
module had a somewhat significant issue where the sorting construction logic only expected to receiveFieldSort
subclasses, whileSmartArtistSort
was not one. ThusSmartArtistSort
has now been forced to behave and is aFieldSort
subclass. It's also been moved toquery.py
module which is where the rest of sorts are defined.