Skip to content

Refactor (typing): add annotations in dialect/parser/schema modules#7444

Open
OutSquareCapital wants to merge 9 commits intotobymao:mainfrom
OutSquareCapital:parser-annotations
Open

Refactor (typing): add annotations in dialect/parser/schema modules#7444
OutSquareCapital wants to merge 9 commits intotobymao:mainfrom
OutSquareCapital:parser-annotations

Conversation

@OutSquareCapital
Copy link
Copy Markdown
Contributor

@OutSquareCapital OutSquareCapital commented Apr 2, 2026

This typing PR annotate numerous functions/methods from core modules with more precise types.

Going from x: list to x: list[Any] or even x: list[object] may seem pointless, but strict type checkers like basedpyright with ALL rules, or pylance with strict mode=on , will complain on non-defined generic types, both at the definition AND call sites (hence for end users of the library).

For example:

image

@georgesittas
Copy link
Copy Markdown
Collaborator

@OutSquareCapital I think wherever we expect args we want to also handle None. There are some places (example) currently where this is not the case. I wonder if we should just do Sequence[t.Any] or Sequence[object] for now to avoid dealing with typing args precisely. What do you think?

@OutSquareCapital
Copy link
Copy Markdown
Contributor Author

If we have a known set of types expected to be used in the Sequence for args, I could try to figure out a solution avoiding Any.
Otherwise yes, Sequence[Any] would be the solution.
(object can't be used here unfortunately since there's "concrete" attribute/method uses at some point)

@georgesittas
Copy link
Copy Markdown
Collaborator

I think it's ok to go for Any now if that's a strict improvement over what we have today. Analyzing the possible values for args and/or possibly restricting them as part of a refactor to get a cleaner typing API can be done as a follow-up to this.

@OutSquareCapital
Copy link
Copy Markdown
Contributor Author

I think that's a good idea.
So, I refactored all the args: Sequence[...] to args: Sequence[Any], with a centralized type alias to clearly mark them, and make future changes easier.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants