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

[noamkush]

I have made things!

This fixes the annotation for database functions which can take F objects and field names, not just an Expression. Also added annotation for the specialized Round constructor.

• Closes accept str for Trunc expression #2196

[intgr]

Per stubtest output, please also remove these entries from allowlist:

note: unused allowlist entry django.db.models.functions.Round.__init__
note: unused allowlist entry django.db.models.functions.math.Round.__init__

[noamkush]

Done

[intgr]

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?

[intgr]

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.

[intgr]

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

[intgr]

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.

[intgr]

Thanks, looks good to me.

Leaving open for a bit, in case someone has thoughts on BaseExpression: #2195 (comment)

[mkurnikov]

Looks good to me, I checked, all possible BaseExpression children also inherit Combinable, either directly, or via Expression base class.
I'd advise to add one test where you could pass every possible Expression child class to one of the places you've annotated as Combinable | str, so we won't miss this in the future.
But it's up to you, could be too much to add every single case. Maybe not every single case, but just one per "unique inheritance chain", like one of the LessThan / GreaterThan and so on.

[intgr]

Thanks for confirming the class hierarchy wrt BaseExpression.

I'm not against adding simple tests, but our usual policy is to not require tests for simple stubs, I'd just merge this as is. Once these hints are fixed, it's very unlikely they will regress again. (The bigger challenge is remembering to use the correct types when new classes are added, but that can't really be covered with current test infra)