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

Lookup should be a subtype of Expression #2199

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions django-stubs/db/models/expressions.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ class BaseExpression:
def __init__(self, output_field: Field | None = ...) -> None: ...
def get_db_converters(self, connection: BaseDatabaseWrapper) -> list[Callable]: ...
def get_source_expressions(self) -> list[Any]: ...
def set_source_expressions(self, exprs: Sequence[Combinable]) -> None: ...
def set_source_expressions(self, exprs: Sequence[Combinable | Expression]) -> None: ...
@cached_property
def contains_aggregate(self) -> bool: ...
@cached_property
Expand All @@ -92,7 +92,7 @@ class BaseExpression:
def convert_value(self) -> Callable: ...
def get_lookup(self, lookup: str) -> type[Lookup] | None: ...
def get_transform(self, name: str) -> type[Transform] | None: ...
def relabeled_clone(self, change_map: dict[str | None, str]) -> Self: ...
def relabeled_clone(self, change_map: Mapping[str, str]) -> Self: ...
def copy(self) -> Self: ...
def get_group_by_cols(self) -> list[BaseExpression]: ...
def get_source_fields(self) -> list[Field | None]: ...
Expand Down
15 changes: 5 additions & 10 deletions django-stubs/db/models/lookups.pyi
Original file line number Diff line number Diff line change
@@ -1,19 +1,18 @@
from collections.abc import Iterable, Mapping
from typing import Any, Generic, Literal, TypeVar
from collections.abc import Iterable
from typing import Any, Generic, Literal, Sequence, TypeVar

from django.core.exceptions import EmptyResultSet
from django.db.backends.base.base import BaseDatabaseWrapper
from django.db.models.expressions import Expression, Func
from django.db.models.expressions import Combinable, Expression, Func
from django.db.models.fields import BooleanField
from django.db.models.query_utils import RegisterLookupMixin
from django.db.models.sql.compiler import SQLCompiler, _AsSqlType, _ParamT
from django.utils.datastructures import OrderedSet
from django.utils.functional import cached_property
from typing_extensions import Self

_T = TypeVar("_T")

class Lookup(Generic[_T]):
class Lookup(Expression, Generic[_T]):
lookup_name: str
prepare_rhs: bool
can_use_none_as_rhs: bool
Expand All @@ -26,16 +25,14 @@ class Lookup(Generic[_T]):
self, compiler: SQLCompiler, connection: BaseDatabaseWrapper, rhs: OrderedSet | None = ...
) -> tuple[list[str], list[str]]: ...
def get_source_expressions(self) -> list[Expression]: ...
def set_source_expressions(self, new_exprs: list[Expression]) -> None: ...
def set_source_expressions(self, new_exprs: Sequence[Combinable | Expression]) -> None: ...
def get_prep_lookup(self) -> Any: ...
def get_db_prep_lookup(self, value: _ParamT, connection: BaseDatabaseWrapper) -> _AsSqlType: ...
def process_lhs(
self, compiler: SQLCompiler, connection: BaseDatabaseWrapper, lhs: Expression | None = ...
) -> _AsSqlType: ...
def process_rhs(self, compiler: SQLCompiler, connection: BaseDatabaseWrapper) -> _AsSqlType: ...
def rhs_is_direct_value(self) -> bool: ...
def relabeled_clone(self, relabels: Mapping[str, str]) -> Self: ...
def get_group_by_cols(self) -> list[Expression]: ...
def as_sql(self, compiler: SQLCompiler, connection: BaseDatabaseWrapper) -> _AsSqlType: ...
def as_oracle(self, compiler: SQLCompiler, connection: BaseDatabaseWrapper) -> _AsSqlType: ...
@cached_property
Expand All @@ -45,8 +42,6 @@ class Lookup(Generic[_T]):
@cached_property
def contains_over_clause(self) -> bool: ...
@property
def is_summary(self) -> bool: ...
@property
def identity(self) -> tuple[type[Lookup], Any, Any]: ...

class Transform(RegisterLookupMixin, Func):
Expand Down
1 change: 0 additions & 1 deletion django-stubs/db/models/sql/query.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,6 @@ class Query(BaseExpression):
def get_meta(self) -> Options: ...
def clone(self) -> Query: ...
def chain(self, klass: type[Query] | None = ...) -> Query: ...
def relabeled_clone(self, change_map: dict[str | None, str]) -> Query: ...
def get_count(self, using: str) -> int: ...
def has_filters(self) -> WhereNode: ...
def has_results(self, using: str) -> bool: ...
Expand Down
6 changes: 0 additions & 6 deletions scripts/stubtest/allowlist_todo.txt
Original file line number Diff line number Diff line change
Expand Up @@ -334,8 +334,6 @@ django.contrib.gis.db.models.Lookup.get_prep_lhs
django.contrib.gis.db.models.Lookup.allowed_default
django.contrib.gis.db.models.Lookup.lookup_name
django.contrib.gis.db.models.Lookup.output_field
django.contrib.gis.db.models.Lookup.relabeled_clone
django.contrib.gis.db.models.Lookup.resolve_expression
django.contrib.gis.db.models.Lookup.select_format
django.contrib.gis.db.models.Manager.__slotnames__
django.contrib.gis.db.models.ManyToManyField.__get__
Expand Down Expand Up @@ -764,8 +762,6 @@ django.db.models.Lookup.get_prep_lhs
django.db.models.Lookup.lookup_name
django.db.models.Lookup.allowed_default
django.db.models.Lookup.output_field
django.db.models.Lookup.relabeled_clone
django.db.models.Lookup.resolve_expression
django.db.models.Lookup.select_format
django.db.models.Manager.__slotnames__
django.db.models.ManyToManyField.__get__
Expand Down Expand Up @@ -1080,8 +1076,6 @@ django.db.models.lookups.Lookup.get_prep_lhs
django.db.models.lookups.Lookup.allowed_default
django.db.models.lookups.Lookup.lookup_name
django.db.models.lookups.Lookup.output_field
django.db.models.lookups.Lookup.relabeled_clone
django.db.models.lookups.Lookup.resolve_expression
django.db.models.lookups.Lookup.select_format
django.db.models.lookups.PatternLookup.process_rhs
django.db.models.manager.BaseManager.aaggregate
Expand Down
10 changes: 10 additions & 0 deletions tests/typecheck/db/models/test_constraints.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,13 @@
main:4: note: Possible overload variants:
main:4: note: .*
main:4: note: .*

- case: model_level_check_contraint_should_allow_lookups
main: |
from django.db.models import CheckConstraint, F
from django.db.models.lookups import LessThan

CheckConstraint(
name="less_than_constraint",
check=LessThan(F("months"), 1)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since mypy typecheck tests are slow, and we also want to test with Pyright, such tests are better added under tests/assert_type

Copy link
Member Author

@mkurnikov mkurnikov May 31, 2024

Choose a reason for hiding this comment

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

Thanks, I should've waited for your review before merging.
I'll fix it in a follow-up.

Copy link
Collaborator

Choose a reason for hiding this comment

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

No problem, this is more of a minor quibble. Follow-up PR is welcome. :)

Welcome back to django-stubs development @mkurnikov! 🎉

Copy link
Collaborator

Choose a reason for hiding this comment

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

FWIW @flaeppe already changed this #2204

Copy link
Member Author

Choose a reason for hiding this comment

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

Great, thanks @flaeppe

)
Loading