Opened 4 years ago

Closed 4 years ago

#24649 closed New feature (fixed)

Allow using the Avg aggregate on non-numeric field types

Reported by: Manuel Dominguez Owned by: nobody
Component: Database layer (models, ORM) Version: master
Severity: Normal Keywords:
Cc: josh.smeaton@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I am using the new DurationField and I tried to query the average value of that field but instead of returning any value the code raises a TypeError. The problem is that the Avg aggregate function tries to cast the returned value to float.

    def convert_value(self, value, expression, connection, context):
        if value is None:
            return value
        return float(value)

As the returned value is a timedelta the casting fails. I do not know if the right way to solve this is to allow to return timedeltas or use a new aggregate function just for DurationFields but I have solved with the latest.

class AvgDuration(Avg):
    def convert_value(self, value, expression, connection, context):
        return value

Change History (7)

comment:1 Changed 4 years ago by Tim Graham

Keywords: Avg DurationField removed
Summary: Avg on DurationField raises TypeError.Allow using the Avg aggregate on non-numeric field types
Triage Stage: UnreviewedAccepted
Type: BugNew feature
Version: 1.8master

The docs for Avg say "Returns the mean value of the given expression, which must be numeric." I am not an expressions expert, but it seems to me we could probably make it more generic based on the value of output_field.

comment:2 Changed 4 years ago by Claude Paroz

Maybe even the convert_value method might be completely removed (as the default is using output_field already). At least, current tests don't seem to scream with that. Josh?

comment:3 Changed 4 years ago by Josh Smeaton

The float(value) is pretty much a direct port of the behaviour before expressions. It does need to be changed in some way though.

I'd have to run some tests, but I think there may be some issues if you try to take the Average of an IntegerField or a DecimalField without running convert_value. I'm not sure that the output_field is used for anything other than backend conversions.

I think we can get rid of the convert_value method, and add an output_field=FloatField into the constructor. Then to take the average of duration field:

.aggregate(Avg('duration', output_field=DurationField()))

Are there any other types that the AVG aggregate works with that doesn't return a float? If there are, we could probably think of a better all-round solution.

comment:4 Changed 4 years ago by Josh Smeaton

Cc: josh.smeaton@… added

comment:5 Changed 4 years ago by Anssi Kääriäinen

Seems to me defaulting to FloatField for output_field is good enough.

comment:6 Changed 4 years ago by Tim Graham

Has patch: set

comment:7 Changed 4 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: newclosed

In 2d76b61:

Fixed #24649 -- Allowed using Avg aggregate on non-numeric field types.

Note: See TracTickets for help on using tickets.
Back to Top