add lots of type hints #6122

amogus07 · 2025-10-24T00:51:26Z

Description

I added type hints ~~and refactored the modules to fix import cycles~~

To Do

Documentation
Changelog
Tests

sourcery-ai

Sorry @amogus07, your pull request is larger than the review limit of 150000 diff characters

github-actions · 2025-10-24T00:51:45Z

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.

semohr · 2025-10-24T09:47:29Z

Hey, thanks a lot for the contribution, really appreciate the effort you’ve put into this!

This PR is quite large, which makes it a bit difficult to review thoroughly. Would you mind breaking it down into smaller, more focused commits or separate PRs? For instance, you could move file relocations or refactors into their own commits to make the changes easier to follow. You can take a look at the PR linked below for an example of how to structure these kinds of refactors.

It also looks like this PR touches on multiple areas of the codebase. To keep things maintainable and easier to review, it’s best if each PR focuses on a single concern or goal, rather than combining several changes at once.

As a side note, have you had a chance to look at #6119? It seems to address some of the same issues you’re working on here, it might be worth aligning with that effort to avoid overlap.

Thanks again for your work on this! We really appreciate your recent efforts!

snejus

Thanks for your contribution!

I reviewed a couple of files but stopped at queryparse.py since I found myself confused about the motivation for these changes?

I'm struggling to find any improvements here, while I see a fair bit off issues. Please see the comments.

snejus · 2025-10-29T04:25:11Z

beets/autotag/distance.py

    process and their access comes with a performance overhead.
    """
-    dist = Distance()
+    dist: Distance = Distance()


This is redundant

snejus · 2025-10-29T04:25:31Z

beets/autotag/distance.py

        self.add(key, dist)

-    def add_data_source(self, before: str | None, after: str | None) -> None:
+    def add_data_source(self, before: object, after: str | None) -> None:


snejus · 2025-10-29T04:27:50Z

beets/autotag/distance.py

+    info_length: float | None
    if info_length := track_info.length:
-        diff = abs(item.length - info_length) - get_track_length_grace()
+        diff: float = (


Again, redundant

snejus · 2025-10-29T04:28:01Z

beets/dbcore/db.py


    @cached_classproperty
-    def _queries(cls) -> dict[str, FieldQueryType]:
+    def _queries(cls) -> dict[str, type[Query]]:


All of these changes from FieldQuery -> Query are incorrect. Why?

snejus · 2025-10-29T04:29:30Z

beets/dbcore/db.py



-class Results(Generic[AnyModel]):
+class Results(Generic[AnyModel, D]):


snejus · 2025-10-29T04:34:01Z

beets/dbcore/queryparse.py

-    r"(?<!\\):"  # Unescaped :
-    r")?"
-    r"(.*)",  # The term itself.
+    r"(-|\^)?"  # Negation prefixes. #  noqa: ISC003


Why? Please undo

snejus · 2025-10-29T04:34:27Z

beets/dbcore/queryparse.py

    # Apply the regular expression and extract the components.
    part = part.strip()
-    match = PARSE_QUERY_PART_REGEX.match(part)
+    match: re.Match[str] | None = PARSE_QUERY_PART_REGEX.match(part)


This is redundant

snejus · 2025-10-29T04:39:20Z

beets/dbcore/queryparse.py

        # If there's no key (field name) specified, this is a "match anything"
        # query.
        out_query = model_cls.any_field_query(pattern, query_class)
-    else:


Why switching the order of the conditional?

Why did you replace FieldQuery in the function input and then introduce cast here?

snejus · 2025-10-29T04:39:32Z

beets/dbcore/queryparse.py

        query_classes[k] = t.query
-    query_classes.update(model_cls._queries)  # Non-field queries.
+    query_classes.update(
+        model_cls._queries.items()  # Non-field queries.


snejus · 2025-10-29T04:42:17Z

beets/dbcore/queryparse.py


 import itertools
 import re
-from typing import TYPE_CHECKING


I don't see any improvements in this module except for redundant types and adjustments in function input types which then force you to use cast and type: ignore.

snejus · 2025-10-29T05:01:55Z

Apologies for my language - I realise I sound dismissive and terse there - I am tired and about to go to sleep, so this is my fault, not your changes <3

I take that most of these type adjustments stem from you using pyright to type check this project. In an ideal world, pyright and mypy would show the same issues, but they don't in reality. This project uses mypy as our type checker, so changes should address mypy's requirements rather than pyright's. Some of these type hints may be fixing issues that pyright sees but mypy doesn't, because it manages to resolve the types.

Would you mind running mypy on your changes to see which type issues actually need addressing for our setup? That would help focus the PR on changes that are truly necessary. Please use resolve_type to see whether a type is missing for a variable.

Thanks again for your effort on this - I really do appreciate it!

amogus07 · 2025-10-29T08:49:10Z

@snejus Thank you for taking the time to review this! Yeah, I probably should've left it as a draft for now, I pretty much expected some changes would have to be reverted. I did realize that this project uses mypy, but either it doesn't play well with my editor (Zed) or just it's LSP isn't as good as Basedpyright, which is what I'm using. As for the "redundant" type annotations, that's just my personal preference. Sure, you can live without them, but using I just don't like the concept of inferred types at all. They can change without me noticing if I accidentally reuse a variable for an unintended purpose. My goal is to get beets closer to a state where it complies with mypy's strict mode. Although I might suggest migrating to a different type checker like basedpyright at some point. On the other hand, we could wait until pyrefly becomes stable, which is significantly faster then both mypy and basedpyright. I will go over your feedback in detail when I have time and use it as guidance for future contributions.

amogus07 requested review from a team and semohr as code owners October 24, 2025 00:51

sourcery-ai bot reviewed Oct 24, 2025

View reviewed changes

amogus07 force-pushed the add-type-hints branch from 86c1c56 to 82b7a1d Compare October 24, 2025 09:31

amogus07 force-pushed the add-type-hints branch from 82b7a1d to c39849f Compare October 24, 2025 12:18

amogus07 changed the title ~~refactor util and ui + add lots of type hints~~ add lots of type hints Oct 24, 2025

amogus07 force-pushed the add-type-hints branch from c39849f to 40b9ad5 Compare October 24, 2025 12:24

add lots of type hints

2ae9270

amogus07 force-pushed the add-type-hints branch from 40b9ad5 to 2ae9270 Compare October 24, 2025 12:27

snejus requested changes Oct 29, 2025

View reviewed changes



		class Results(Generic[AnyModel]):
		class Results(Generic[AnyModel, D]):

add lots of type hints #6122

Are you sure you want to change the base?

add lots of type hints #6122

Uh oh!

Conversation

amogus07 commented Oct 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

To Do

Uh oh!

sourcery-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Oct 24, 2025

Uh oh!

semohr commented Oct 24, 2025

Uh oh!

snejus left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

snejus commented Oct 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

amogus07 commented Oct 29, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

amogus07 commented Oct 24, 2025 •

edited

Loading

snejus commented Oct 29, 2025 •

edited

Loading