-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Move queries and types to respective modules #5775
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
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. |
49f6be1
to
39cc0ee
Compare
c8cb902
to
219aed9
Compare
219aed9
to
a53383c
Compare
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.
Not a thorough review, just a few initial comments.
In general, this PR feels too me like it starts to erode the separation between high-level library
code and low-level dbcore
a little (Consider for example the MusicalKey
type). I think @sampsyo mentioned somewhere that the original intent was to keep the latter as a fully re-usable and generic module, without any specifics for music libraries. Usage of dbcore
for anything but Item
and Album
never happened, though.
I haven't really formed a clear opinion yet on where the ideal abstraction boundary lies here.
1a1bbbc
to
f0ba201
Compare
38bd19f
to
0133f06
Compare
0133f06
to
0e04f7b
Compare
27e444d
to
45d65e8
Compare
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.
Pull Request Overview
This PR refactors the project by extracting query and type definitions from library.py
into the dbcore
package, centralizes utility functions in a new beets.util.units
module, and updates imports and tests to match.
- Query and Type Relocation: Moved
PathQuery
,SingletonQuery
tobeets.dbcore.query
and database types (DateType
,PathType
, etc.) tobeets.dbcore.types
. - Utilities Consolidation: Extracted
human_bytes
,human_seconds
, andhuman_seconds_short
intobeets.util.units
and updated UI code accordingly. - Test Modernization: Replaced legacy unittest-based tests with pytest parametrization, removed duplicates, and updated imports.
Reviewed Changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
test/util/test_units.py | Added pytest tests for relocated byte and time formatting helpers. |
test/test_ui_init.py | Removed duplicated UI formatting tests and cleaned up imports. |
test/test_types.py | Added tests for core type classes in dbcore.types . |
test/test_query.py | Dropped old PathQueryTest , wrote new parametrized pytest tests. |
test/test_library.py | Removed outdated library-specific type tests. |
beetsplug/web/init.py | Swapped beets.library.PathQuery for dbcore.query.PathQuery . |
beetsplug/types.py | Replaced library.DateType() with types.DATE and removed import. |
beetsplug/spotify.py | Switched to types.DATE and removed now-unused import. |
beetsplug/playlist.py | Consolidated BLOB_TYPE and InQuery imports from dbcore.query . |
beetsplug/mpdstats.py | Updated PathQuery and date-type references to dbcore . |
beetsplug/metasync/itunes.py | Replaced legacy DateType with types.DATE . |
beetsplug/metasync/amarok.py | Ditto for DateType . |
beetsplug/deezer.py | Ditto for DateType . |
beets/util/units.py | New module defining human-readable size/time formatting funcs. |
beets/util/init.py | Removed duplicate formatting functions now in units.py . |
beets/ui/commands.py | Updated imports to use relocated formatting functions. |
beets/ui/init.py | Removed legacy formatting helpers. |
beets/library.py | Removed inline query/type classes and updated all references. |
beets/dbcore/types.py | Introduced core type definitions extracted from library.py . |
beets/dbcore/query.py | Introduced core query definitions extracted from library.py . |
.git-blame-ignore-revs | Added revision to ignore bulk copy of queries/types. |
45d65e8
to
95a5e4b
Compare
Thank you for the PR! I really appreciate it, this will make our lives much easier in the long run! |
95a5e4b
to
1dee62d
Compare
And remove `force_implicit_query_detection` attribute from `PathQuery` class.
The case_sensitive parameter was only used in tests, which now use monkeypatch to control the behavior of util.case_sensitive() instead. This simplifies the PathQuery initialization logic while maintaining test coverage.
1dee62d
to
9d088ab
Compare
This PR moves query and type definitions away from
library.py
todbcore
to improve modularity and organization.Key Changes:
PathQuery
andSingletonQuery
moved frombeets.library
tobeets.dbcore.query
.DateType
,PathType
(and its variantsNullPathType
),MusicalKey
, andDurationType
moved frombeets.library
tobeets.dbcore.types
.BLOB_TYPE
definition was moved frombeets.library
tobeets.dbcore.query
and then referenced inbeets.dbcore.types
.human_seconds_short
utility function was moved frombeets.ui
tobeets.util
due to circular dependency.PathQueryTest
class intest/test_query.py
has been rewritten to usepytest.mark.parametrize
for more concise and readable test cases.