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.

Sign up for GitHub

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

Update django.db.models.functions: allow Combinable as parameter, add Round function #2195

Merged
merged 2 commits into from
May 31, 2024
Merged

Update django.db.models.functions: allow Combinable as parameter, add Round function #2195

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e01db53
Fix database function annotations.
noamk-hl May 29, 2024
22bb12b
Update allowlist_todo.txt
noamkush May 29, 2024
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
5 changes: 3 additions & 2 deletions django-stubs/db/models/functions/comparison.pyi
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,17 @@
from typing import Any, ClassVar

from django.db.models import Func
from django.db.models.expressions import Combinable
from django.db.models.fields import Field
from django.db.models.fields.json import JSONField

class Cast(Func):
def __init__(self, expression: Any, output_field: str | Field) -> None: ...
def __init__(self, expression: Combinable | str, output_field: str | Field) -> None: ...

class Coalesce(Func): ...

class Collate(Func):
def __init__(self, expression: Any, collation: str) -> None: ...
def __init__(self, expression: Combinable | str, collation: str) -> None: ...

class Greatest(Func): ...

Expand Down
8 changes: 4 additions & 4 deletions django-stubs/db/models/functions/datetime.pyi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ from typing import Any, ClassVar
from django.db import models
from django.db.backends.base.base import BaseDatabaseWrapper
from django.db.models import Func, Transform
from django.db.models.expressions import Expression
from django.db.models.expressions import Combinable
from django.db.models.fields import Field
from django.db.models.sql.compiler import SQLCompiler, _AsSqlType

Expand All @@ -16,7 +16,7 @@ class Extract(TimezoneMixin, Transform):
lookup_name: str
output_field: ClassVar[models.IntegerField]
def __init__(
self, expression: Any, lookup_name: str | None = ..., tzinfo: Any | None = ..., **extra: Any
self, expression: Combinable | str, lookup_name: str | None = ..., tzinfo: Any | None = ..., **extra: Any
) -> None: ...

class ExtractYear(Extract): ...
Expand All @@ -41,14 +41,14 @@ class TruncBase(TimezoneMixin, Transform):
tzinfo: Any

def __init__(
self, expression: Expression, output_field: Field | None = ..., tzinfo: tzinfo | None = ..., **extra: Any
self, expression: Combinable | str, output_field: Field | None = ..., tzinfo: tzinfo | None = ..., **extra: Any
Copy link
Collaborator

@intgr intgr May 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I'm not very familiar QuerySet operators/functions.

Most Combinables seem to inherit from Expression. Can you give an example of something that is Combinable but not Expression?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also some PostgreSQL functions are typehinted as BaseExpression | Combinable | str. I wonder if there's a reason for that, or in practice all useful expressions already inherit from Combinable.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sry, you already answered the first question in PR description. F() objects inherit from Combinable but not Expression.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if there's a reason for that, or in practice all useful expressions already inherit from Combinable.

But unless someone can think of a use case for BaseExpression, I'm inclined to go with Combinable | str. Let's not complicate things unless we know it's necessary.

) -> None: ...
def as_sql(self, compiler: SQLCompiler, connection: BaseDatabaseWrapper) -> _AsSqlType: ... # type: ignore[override]

class Trunc(TruncBase):
def __init__(
self,
expression: Expression,
expression: Combinable | str,
kind: str,
output_field: Field | None = ...,
tzinfo: tzinfo | None = ...,
Expand Down
9 changes: 7 additions & 2 deletions django-stubs/db/models/functions/math.pyi
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
from django.db.models.expressions import Func
from typing import Any

from django.db.models.expressions import Combinable, Func
from django.db.models.functions.mixins import FixDecimalInputMixin, NumericOutputFieldMixin
from django.db.models.lookups import Transform

Expand All @@ -19,7 +21,10 @@ class Mod(FixDecimalInputMixin, NumericOutputFieldMixin, Func): ...
class Pi(NumericOutputFieldMixin, Func): ...
class Power(NumericOutputFieldMixin, Func): ...
class Radians(NumericOutputFieldMixin, Transform): ...
class Round(Transform): ...

class Round(Transform):
def __init__(self, expression: Combinable | str, precision: int = ..., **extra: Any) -> None: ...

class Sin(NumericOutputFieldMixin, Transform): ...
class Sqrt(NumericOutputFieldMixin, Transform): ...
class Tan(NumericOutputFieldMixin, Transform): ...
Expand Down
8 changes: 4 additions & 4 deletions django-stubs/db/models/functions/text.pyi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ class Concat(Func):

class Left(Func):
output_field: ClassVar[models.CharField]
def __init__(self, expression: Expression | str, length: Expression | int, **extra: Any) -> None: ...
def __init__(self, expression: Combinable | str, length: Expression | int, **extra: Any) -> None: ...
def get_substr(self) -> Substr: ...
def as_oracle(self, compiler: SQLCompiler, connection: BaseDatabaseWrapper, **extra_context: Any) -> _AsSqlType: ...
def as_sqlite(self, compiler: SQLCompiler, connection: BaseDatabaseWrapper, **extra_context: Any) -> _AsSqlType: ...
Expand All @@ -46,7 +46,7 @@ class Lower(Transform): ...
class LPad(Func):
output_field: ClassVar[models.CharField]
def __init__(
self, expression: Expression | str, length: Expression | int | None, fill_text: Expression = ..., **extra: Any
self, expression: Combinable | str, length: Expression | int | None, fill_text: Expression = ..., **extra: Any
) -> None: ...

class LTrim(Transform): ...
Expand All @@ -59,7 +59,7 @@ class Ord(Transform):

class Repeat(Func):
output_field: ClassVar[models.CharField]
def __init__(self, expression: Expression | str, number: Expression | int | None, **extra: Any) -> None: ...
def __init__(self, expression: Combinable | str, number: Expression | int | None, **extra: Any) -> None: ...
def as_oracle(self, compiler: SQLCompiler, connection: BaseDatabaseWrapper, **extra_context: Any) -> _AsSqlType: ...

class Replace(Func):
Expand Down Expand Up @@ -91,7 +91,7 @@ class StrIndex(Func):
class Substr(Func):
output_field: ClassVar[models.CharField]
def __init__(
self, expression: Expression | str, pos: Expression | int, length: Expression | int | None = ..., **extra: Any
self, expression: Combinable | str, pos: Expression | int, length: Expression | int | None = ..., **extra: Any
) -> None: ...
def as_sqlite(self, compiler: SQLCompiler, connection: BaseDatabaseWrapper, **extra_context: Any) -> _AsSqlType: ...
def as_oracle(self, compiler: SQLCompiler, connection: BaseDatabaseWrapper, **extra_context: Any) -> _AsSqlType: ...
Expand Down
2 changes: 0 additions & 2 deletions scripts/stubtest/allowlist_todo.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1042,7 +1042,6 @@ django.db.models.functions.NullIf.as_oracle
django.db.models.functions.Pi.as_oracle
django.db.models.functions.Radians.as_oracle
django.db.models.functions.Random
django.db.models.functions.Round.__init__
django.db.models.functions.comparison.Cast.as_mysql
django.db.models.functions.comparison.Cast.as_oracle
django.db.models.functions.comparison.Cast.as_postgresql
Expand All @@ -1066,7 +1065,6 @@ django.db.models.functions.math.Degrees.as_oracle
django.db.models.functions.math.Pi.as_oracle
django.db.models.functions.math.Radians.as_oracle
django.db.models.functions.math.Random
django.db.models.functions.math.Round.__init__
django.db.models.functions.mixins.FixDecimalInputMixin.as_postgresql
django.db.models.functions.mixins.FixDurationInputMixin.as_mysql
django.db.models.functions.mixins.FixDurationInputMixin.as_oracle
Expand Down
Loading