Skip to content

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

Merged
merged 6 commits into from
Jul 6, 2025

Conversation

snejus
Copy link
Member

@snejus snejus commented May 11, 2025

This PR moves query and type definitions away from library.py to dbcore to improve modularity and organization.

Key Changes:

  • Query and Type Relocation:
    • PathQuery and SingletonQuery moved from beets.library to beets.dbcore.query.
    • DateType, PathType (and its variants NullPathType), MusicalKey, and DurationType moved from beets.library to beets.dbcore.types.
    • The BLOB_TYPE definition was moved from beets.library to beets.dbcore.query and then referenced in beets.dbcore.types.
    • The human_seconds_short utility function was moved from beets.ui to beets.util due to circular dependency.
  • Test Modernization:
    • The PathQueryTest class in test/test_query.py has been rewritten to use pytest.mark.parametrize for more concise and readable test cases.
  • Import Updates: All internal references to these moved classes and functions have been updated across the codebase.

@snejus snejus requested review from Copilot, semohr and wisp3rwind May 11, 2025 18:11
Copy link

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.

@snejus snejus changed the title Move queries types to respective modules Move queries and types to respective modules May 11, 2025
Copilot

This comment was marked as outdated.

@snejus snejus force-pushed the move-queries-types-to-respective-modules branch 4 times, most recently from 49f6be1 to 39cc0ee Compare May 12, 2025 08:50
@snejus snejus force-pushed the move-queries-types-to-respective-modules branch 2 times, most recently from c8cb902 to 219aed9 Compare May 12, 2025 12:46
@snejus snejus force-pushed the move-queries-types-to-respective-modules branch from 219aed9 to a53383c Compare May 26, 2025 14:42
Copy link
Member

@wisp3rwind wisp3rwind left a 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.

@snejus snejus force-pushed the move-queries-types-to-respective-modules branch 8 times, most recently from 1a1bbbc to f0ba201 Compare June 1, 2025 14:28
@snejus snejus force-pushed the move-queries-types-to-respective-modules branch 4 times, most recently from 38bd19f to 0133f06 Compare June 22, 2025 17:07
@snejus snejus force-pushed the move-queries-types-to-respective-modules branch from 0133f06 to 0e04f7b Compare July 4, 2025 07:49
@snejus snejus mentioned this pull request Jul 5, 2025
@snejus snejus force-pushed the move-queries-types-to-respective-modules branch from 27e444d to 45d65e8 Compare July 5, 2025 19:41
@snejus snejus requested review from Copilot and semohr July 5, 2025 19:41
Copy link
Contributor

@Copilot Copilot AI left a 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 to beets.dbcore.query and database types (DateType, PathType, etc.) to beets.dbcore.types.
  • Utilities Consolidation: Extracted human_bytes, human_seconds, and human_seconds_short into beets.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.

@snejus snejus force-pushed the move-queries-types-to-respective-modules branch from 45d65e8 to 95a5e4b Compare July 5, 2025 19:46
@semohr
Copy link
Contributor

semohr commented Jul 6, 2025

Thank you for the PR! I really appreciate it, this will make our lives much easier in the long run!

@snejus snejus force-pushed the move-queries-types-to-respective-modules branch from 95a5e4b to 1dee62d Compare July 6, 2025 15:03
snejus added 5 commits July 6, 2025 16:09
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.
@snejus snejus force-pushed the move-queries-types-to-respective-modules branch from 1dee62d to 9d088ab Compare July 6, 2025 15:10
@snejus snejus enabled auto-merge July 6, 2025 15:10
@snejus snejus merged commit 7165b04 into master Jul 6, 2025
18 checks passed
@snejus snejus deleted the move-queries-types-to-respective-modules branch July 6, 2025 15:14
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.

3 participants