Skip to content
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

[flake8-type-checking] TC004 is too strict for SQLAlchemy's Mapped #16412

Open
Daverball opened this issue Feb 27, 2025 · 0 comments
Open

[flake8-type-checking] TC004 is too strict for SQLAlchemy's Mapped #16412

Daverball opened this issue Feb 27, 2025 · 0 comments
Labels
bug Something isn't working

Comments

@Daverball
Copy link
Contributor

Daverball commented Feb 27, 2025

Summary

Given something like:

https://play.ruff.rs/da4c76c4-92d0-4083-99c1-022152398a50

from __future__ import annotations

from sqlalchemy.orm import BaseModel, Mapped, relationship
from typing import TYPE_CHECKING

if TYPE_CHECKING:
    from .bar import Bar

class Foo(BaseModel):
    id: Mapped[int]
    bar_id: Mapped[int | None]
    bar: Mapped[Bar | None] = relationship()

Ruff will flag the import inside the TYPE_CHECKING block, which very well may be required, due to circular dependencies between the two models.

Additionally runtime-required-base-classes is technically too strict for sqlalchemy.orm.BaseModel, since only the annotations that use a Mapped class are runtime required and furthermore only non-model types are runtime required, any forward references to other models can be resolved during mapping.

We could handle the latter using an additional setting runtime-required-generic-subscripts. But the former would still be a problem.

To handle the former we would need something like runtime-ambiguous instead of runtime-required, so we emit neither TC001-003 nor TC004 for uses in that context, since the use will have to be considered neither runtime-only nor typing-only.

But turning two settings, into six settings doesn't seem particularly elegant either. Maybe someone can think of a more elegant way to structure these settings.

I think this is the final major hurdle for covering all the use-cases the flake8 plugin currently covers (beyond implementing the currently still missing TC009 rule).

Eventually type inference could let us do something more targeted for SQLAlchemy that knows exactly which symbols aren't runtime-required, but that's still a long way off and not as flexible for other libraries that may do something similar.

Version

0.9.7

@MichaReiser MichaReiser added the bug Something isn't working label Feb 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants