Allow class types for output_field of expressions?

Django Internals
1

A common mistake I see people doing is using output_field with the field type instead of an instance:

Cast('some_field', output_field=BooleanField)

This is incorrect because output_field expects an instance.

Would there be any interest in allowing output_field to accept types and automatically instantiate the type if so? Naturally this would only make sense for fields that don’t require any arguments.

2

What happens when people make this mistake? Do they always immediately get a traceback or can we silently do the wrong thing?

If it’s easy to diagnose and fix for a user, it might be reasonable for Django to wontfix. That said, the added complexity in the Django codebase sounds like it would be minimal, so maybe it’s worth smoothing the experience here.

3

It’s worth noting that type checking can detect this issue, which makes me err towards wontfix because there’s already a tool to detect the problem.

That said it sounds like an easy change (but perhaps it’s not).

4

I thought this too. I know we’re not type checking (yet) but static analysis has got to be better than extra code paths for this (I’d guess).

(This kind of case might be a good candidate for leaf annotations maybe… :thinking:)

5

I have myself made this mistake several times. This post made me dedicate a few minutes to wonder why, whether it’s something (missing or indicating otherwise) in the docs, or something else.

While I (sometimes) need to read (docs) slower to catch this subtleties, I think the Django docs could benefit from the following:

  • Add an introductory paragraph, below “Supported arithmetic”, dedicated to output_field specifics. This could build from the existing paragraph shown in Value or in Aggregate.

The reason for my suggestion is that many of the functions in the referenced doc page use output_field but none except Value and Aggregate describe the param and that’s midway the page content. Specifically, the first time is encountered (when reading top bottom) is in the F docs and is completely out of the blue:

Since F() does not directly support output_field you will need to wrap the expression with ExpressionWrapper

2 Likes
6

@nessita +1. I think the docs throughout this section are a little terse (or not easy to break into) so clarification is definitely welcome from my POV.

7

I’m up for docs changes.

Try django-stubs, come on in, the water is fine!

1 Like
8

Ok cheers avoiding additional code paths when static analysis is an option sounds better.

9

I started this PR to get the conversation going about this docs improvement:

1 Like