Skip to content

refactor: Improve type annotations and use python 3.10+ syntax#14943

Closed
argoarsiks wants to merge 1 commit into
fastapi:masterfrom
argoarsiks:refactor/type-annotations
Closed

refactor: Improve type annotations and use python 3.10+ syntax#14943
argoarsiks wants to merge 1 commit into
fastapi:masterfrom
argoarsiks:refactor/type-annotations

Conversation

@argoarsiks
Copy link
Copy Markdown
Contributor

  • ANN204: add missing return type annotations in fastapi/security/
  • UP038: use X | Y syntax for isinstance calls

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Feb 19, 2026

Merging this PR will not alter performance

✅ 20 untouched benchmarks


Comparing argoarsiks:refactor/type-annotations (1f89f7b) with master (faee822)1

Open in CodSpeed

Footnotes

  1. No successful run was found on master (c441583) during the generation of this report, so faee822 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

Comment thread fastapi/dependencies/utils.py Outdated


def is_union_of_base_models(field_type: Any) -> bool:
def is_union_of_base_models(field_type: type) -> bool:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mypy check...............................................................Failed
- hook id: local-mypy
- exit code: 1

  fastapi/dependencies/utils.py:866: error: Argument 1 to "is_union_of_base_models" has incompatible type "type[Any] | None"; expected "type"  [arg-type]
  Found 1 error in 1 file (checked 47 source files)

@argoarsiks argoarsiks force-pushed the refactor/type-annotations branch from 5f006e5 to 1f89f7b Compare February 19, 2026 13:11
Copy link
Copy Markdown
Member

@YuriiMotov YuriiMotov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@argoarsiks, thanks for your interest!

I think we don't need these changes..

scheme_name: str | None,
auto_error: bool,
):
) -> None:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not necessary to specify return type annotation for __init__ methods.
If we want to enforce this, we should enable such rule for linter

arg
for arg in annotated_args[1:]
if isinstance(arg, (FieldInfo, params.Depends))
if isinstance(arg, FieldInfo | params.Depends)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this syntax, performance decreases.
See astral-sh/ruff#7871



def is_union_of_base_models(field_type: Any) -> bool:
def is_union_of_base_models(field_type: type | None) -> bool:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is necessary. In utils.py we still pass first_field.field_info.annotation which is of type Any

@argoarsiks argoarsiks closed this Feb 23, 2026
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