Re: [Django] #34808: Some aggregation functions may return None; this isn't well documented

[Django]

#34808: Some aggregation functions may return None; this isn't well documented
-+-
 Reporter:  Eric Baumgartner |Owner:  Lufafa
 Type:   |  Joshua
  Cleanup/optimization   |   Status:  closed
Component:  Documentation|  Version:  4.2
 Severity:  Normal   |   Resolution:  fixed
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Natalia <124304+nessita@…>):

 In [changeset:"e8fe48d3a02dbe3b57a87b334f6335b701f0306d" e8fe48d]:
 {{{
 #!CommitTicketReference repository=""
 revision="e8fe48d3a02dbe3b57a87b334f6335b701f0306d"
 [4.2.x] Fixed #34808 -- Doc'd aggregate function's default argument.

 Backport of 8adc7c86ab85ed91e512bc49056e301cbe1715d0 from main
 }}}

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018b2026cee9-e75b84c5-4bb9-4be4-8bab-2f8b95f66794-00%40eu-central-1.amazonses.com.

Re: [Django] #34808: Some aggregation functions may return None; this isn't well documented

[Django]

#34808: Some aggregation functions may return None; this isn't well documented
-+-
 Reporter:  Eric Baumgartner |Owner:  Lufafa
 Type:   |  Joshua
  Cleanup/optimization   |   Status:  closed
Component:  Documentation|  Version:  4.2
 Severity:  Normal   |   Resolution:  fixed
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Natalia <124304+nessita@…>):

 In [changeset:"d4bbdf5337089925f2900c44d3e953a947e4ca0e" d4bbdf53]:
 {{{
 #!CommitTicketReference repository=""
 revision="d4bbdf5337089925f2900c44d3e953a947e4ca0e"
 [5.0.x] Fixed #34808 -- Doc'd aggregate function's default argument.

 Backport of 8adc7c86ab85ed91e512bc49056e301cbe1715d0 from main
 }}}

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018b2026169a-f494fa85-65c5-463f-bac0-472c18ef8258-00%40eu-central-1.amazonses.com.

Re: [Django] #34808: Some aggregation functions may return None; this isn't well documented

[Django]

#34808: Some aggregation functions may return None; this isn't well documented
-+-
 Reporter:  Eric Baumgartner |Owner:  Lufafa
 Type:   |  Joshua
  Cleanup/optimization   |   Status:  closed
Component:  Documentation|  Version:  4.2
 Severity:  Normal   |   Resolution:  fixed
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Natalia <124304+nessita@…>):

 * status:  assigned => closed
 * resolution:   => fixed


Comment:

 In [changeset:"8adc7c86ab85ed91e512bc49056e301cbe1715d0" 8adc7c86]:
 {{{
 #!CommitTicketReference repository=""
 revision="8adc7c86ab85ed91e512bc49056e301cbe1715d0"
 Fixed #34808 -- Doc'd aggregate function's default argument.
 }}}

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018b20251f00-89b1b66a-745d-4b62-a0ca-3765fb4573cf-00%40eu-central-1.amazonses.com.

Re: [Django] #34808: Some aggregation functions may return None; this isn't well documented

[Django]

#34808: Some aggregation functions may return None; this isn't well documented
-+-
 Reporter:  Eric Baumgartner |Owner:  Lufafa
 Type:   |  Joshua
  Cleanup/optimization   |   Status:  assigned
Component:  Documentation|  Version:  4.2
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Lufafa Joshua):

 ~~https://github.com/django/django/pull/17253~~

 new PR https://github.com/django/django/pull/17308

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018ad0990082-55ccf255-ec36-4133-90f5-5025cde736cc-00%40eu-central-1.amazonses.com.

Re: [Django] #34808: Some aggregation functions may return None; this isn't well documented

[Django]

#34808: Some aggregation functions may return None; this isn't well documented
-+-
 Reporter:  Eric Baumgartner |Owner:  Lufafa
 Type:   |  Joshua
  Cleanup/optimization   |   Status:  assigned
Component:  Documentation|  Version:  4.2
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Natalia <124304+nessita@…>):

 In [changeset:"b08f53ff46238301431084b50762a40170d7869d" b08f53f]:
 {{{
 #!CommitTicketReference repository=""
 revision="b08f53ff46238301431084b50762a40170d7869d"
 [4.2.x] Refs #34808 -- Doc'd that aggregation functions on empty groups
 can return None.

 Backport of 78b5c9075348aa12da2e024f6ece29d1d652dfdd from main
 }}}

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018ab8894493-99c06664-870b-442a-a563-d2d78c92add4-00%40eu-central-1.amazonses.com.

Re: [Django] #34808: Some aggregation functions may return None; this isn't well documented

[Django]

#34808: Some aggregation functions may return None; this isn't well documented
-+-
 Reporter:  Eric Baumgartner |Owner:  Lufafa
 Type:   |  Joshua
  Cleanup/optimization   |   Status:  assigned
Component:  Documentation|  Version:  4.2
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Natalia <124304+nessita@…>):

 In [changeset:"fb5dd118e955e34c84effc295aaabcec12e6295b" fb5dd118]:
 {{{
 #!CommitTicketReference repository=""
 revision="fb5dd118e955e34c84effc295aaabcec12e6295b"
 [5.0.x] Refs #34808 -- Doc'd that aggregation functions on empty groups
 can return None.

 Backport of 78b5c9075348aa12da2e024f6ece29d1d652dfdd from main
 }}}

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018ab888adf1-b9df5883-55d1-4214-ac7d-9901167a5206-00%40eu-central-1.amazonses.com.

Re: [Django] #34808: Some aggregation functions may return None; this isn't well documented

[Django]

#34808: Some aggregation functions may return None; this isn't well documented
-+-
 Reporter:  Eric Baumgartner |Owner:  Lufafa
 Type:   |  Joshua
  Cleanup/optimization   |   Status:  assigned
Component:  Documentation|  Version:  4.2
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Natalia <124304+nessita@…>):

 In [changeset:"78b5c9075348aa12da2e024f6ece29d1d652dfdd" 78b5c907]:
 {{{
 #!CommitTicketReference repository=""
 revision="78b5c9075348aa12da2e024f6ece29d1d652dfdd"
 Refs #34808 -- Doc'd that aggregation functions on empty groups can return
 None.
 }}}

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018ab887716b-e96d47f6-1ece-43ea-bc3b-a455aae031bc-00%40eu-central-1.amazonses.com.

Re: [Django] #34808: Some aggregation functions may return None; this isn't well documented

[Django]

#34808: Some aggregation functions may return None; this isn't well documented
-+-
 Reporter:  Eric Baumgartner |Owner:  Lufafa
 Type:   |  Joshua
  Cleanup/optimization   |   Status:  assigned
Component:  Documentation|  Version:  4.2
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Eric Baumgartner):

 >Hey guys, would it be necessary to add a possible return of None on every
 aggregate function

 I'm curious what others think, but I'm pretty firm that the documentation
 for specific aggregation functions like **Sum** should account for
 **None** as a possible return value.

 I know this may seem redundant given that **default** is already
 documented under Aggregation functions generally.

 However, the way some people use the documentation means that they won't
 see the general aggregation documentation, because that's not what they're
 looking for.

 Example (which is pretty close to my experience, which is what led me to
 file this issue):

 - I want to calculate the total of some field across the records in a
 queryset.
 - I'm pretty sure this will be called Sum, so I hit the documentation to
 look up that function.
 - On the querysets page, I scan down the Contents sidebar on the right.
 - I find **Sum** in the sidebar. Great! I click it.

 I'm now scrolled directly to the definition of the Sum function, which
 says:

 >Return type: same as input field, or output_field if supplied

 Why would I not take that at face value? There's nothing in the definition
 of Sum that would lead me to scroll up the page to see the general
 documentation for aggregation functions.

 If I'm calling Sum for an IntegerField, the return type is int. I
 interpret that line as equivalent to `Sum(...) -> int`. When in reality it
 is `Sum(...) -> Optional[int]`.

 I think a relatively simple fix would be to change this to:

 >Return type: same as input field, or output_field if supplied. Returns
 **None** for empty querysets.

 I would suggest adding this for all aggregation functions except `Count`.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018ab3e19c9a-24668ee2-f216-4eda-ab75-ef5ab1222ace-00%40eu-central-1.amazonses.com.

Re: [Django] #34808: Some aggregation functions may return None; this isn't well documented

[Django]

#34808: Some aggregation functions may return None; this isn't well documented
-+-
 Reporter:  Eric Baumgartner |Owner:  Lufafa
 Type:   |  Joshua
  Cleanup/optimization   |   Status:  assigned
Component:  Documentation|  Version:  4.2
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Lufafa Joshua):

 Hey guys, would it be necessary to add a possible return of None on every
 aggregate function in
 https://docs.djangoproject.com/en/4.2/ref/models/querysets/. There is a
 notice under aggregation functions on the querysets page that already does
 so, perhaps adding a link on the querysets page about the default argument
 usage in https://docs.djangoproject.com/en/4.2/topics/db/aggregation/
 would be helpful.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018ab1c29c56-e4c08e10-8c37-4a66-b2a4-efb86a6077d2-00%40eu-central-1.amazonses.com.

Re: [Django] #34808: Some aggregation functions may return None; this isn't well documented

[Django]

#34808: Some aggregation functions may return None; this isn't well documented
-+-
 Reporter:  Eric Baumgartner |Owner:  Lufafa
 Type:   |  Joshua
  Cleanup/optimization   |   Status:  assigned
Component:  Documentation|  Version:  4.2
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-

Comment (by Lufafa Joshua):

 new [# PR] https://github.com/django/django/pull/17253

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018aa2b1681d-bb5de864-dae2-4309-a2bd-b38431890340-00%40eu-central-1.amazonses.com.

Re: [Django] #34808: Some aggregation functions may return None; this isn't well documented

[Django]

#34808: Some aggregation functions may return None; this isn't well documented
-+-
 Reporter:  Eric Baumgartner |Owner:  Lufafa
 Type:   |  Joshua
  Cleanup/optimization   |   Status:  assigned
Component:  Documentation|  Version:  4.2
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-

Comment (by David Sanders):

 Eric, there's a PR up if you'd like to provide some feedback – since you
 had a few points.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018a8e2a6793-20341f3c-0aec-491e-8820-90cdbae53b79-00%40eu-central-1.amazonses.com.

Re: [Django] #34808: Some aggregation functions may return None; this isn't well documented

[Django]

#34808: Some aggregation functions may return None; this isn't well documented
-+-
 Reporter:  Eric Baumgartner |Owner:  Lufafa
 Type:   |  Joshua
  Cleanup/optimization   |   Status:  assigned
Component:  Documentation|  Version:  4.2
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  1
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Mariusz Felisiak):

 * needs_better_patch:  0 => 1


-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018a8d24c201-d70103b3-8a62-4a46-a74f-88908f0adac7-00%40eu-central-1.amazonses.com.

Re: [Django] #34808: Some aggregation functions may return None; this isn't well documented

[Django]

#34808: Some aggregation functions may return None; this isn't well documented
-+-
 Reporter:  Eric Baumgartner |Owner:  Lufafa
 Type:   |  Joshua
  Cleanup/optimization   |   Status:  assigned
Component:  Documentation|  Version:  4.2
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Lufafa Joshua):

 * has_patch:  0 => 1


Comment:

 [# PR]=https://github.com/django/django/pull/17251

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018a8d1c939d-1cafdcfd-d37d-4933-8ce9-6ed479c266f4-00%40eu-central-1.amazonses.com.

Re: [Django] #34808: Some aggregation functions may return None; this isn't well documented

[Django]

#34808: Some aggregation functions may return None; this isn't well documented
-+-
 Reporter:  Eric Baumgartner |Owner:  Lufafa
 Type:   |  Joshua
  Cleanup/optimization   |   Status:  assigned
Component:  Documentation|  Version:  4.2
 Severity:  Normal   |   Resolution:
 Keywords:   | Triage Stage:  Accepted
Has patch:  0|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Lufafa Joshua):

 * owner:  nobody => Lufafa Joshua
 * status:  new => assigned


-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018a70bd1a8d-de3a49d6-6268-456a-a78c-9897cf5740b2-00%40eu-central-1.amazonses.com.

Re: [Django] #34808: Some aggregation functions may return None; this isn't well documented

[Django]

#34808: Some aggregation functions may return None; this isn't well documented
--+
 Reporter:  Eric Baumgartner  |Owner:  nobody
 Type:  Cleanup/optimization  |   Status:  new
Component:  Documentation |  Version:  4.2
 Severity:  Normal|   Resolution:
 Keywords:| Triage Stage:  Accepted
Has patch:  0 |  Needs documentation:  0
  Needs tests:  0 |  Patch needs improvement:  0
Easy pickings:  0 |UI/UX:  0
--+

Comment (by Eric Baumgartner):

 Replying to [comment:5 David Sanders]:
 > Hi Lufafa,
 >
 > It'd be best to check that Eric hasn't already started… (please read the
 comments above).

 I haven't started on this, Lufafa you're welcome to dig in if you want.
 Otherwise I'll try to take a pass on this over the weekend.

 Based on David's feedback I'm backing off the idea of adding default
 params to every use of .aggregate() site-wide and just focusing on the
 examples on the aggregation topic page.

 On the querysets page, I would mention that Aggregate.defaults uses
 Coalesce. And I'm also thinking that the "Return type" documentation for
 each individual function (Sum, Avg, etc) needs to mention the case where
 it may return None. While there is a note about empty querysets directly
 under the Aggregation functions header, I think the way documentation is
 used is that a lot of people scan the list of functions in the sidebar and
 click the one they're interested in. This scrolls them directly to the
 function documentation (e.g. for Sum) and they'll never see the empty
 queryset note.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018a6bf5afd8-faa9f6f2-796c-4eb4-9073-5190747b861d-00%40eu-central-1.amazonses.com.

Re: [Django] #34808: Some aggregation functions may return None; this isn't well documented

[Django]

#34808: Some aggregation functions may return None; this isn't well documented
--+
 Reporter:  Eric Baumgartner  |Owner:  nobody
 Type:  Cleanup/optimization  |   Status:  new
Component:  Documentation |  Version:  4.2
 Severity:  Normal|   Resolution:
 Keywords:| Triage Stage:  Accepted
Has patch:  0 |  Needs documentation:  0
  Needs tests:  0 |  Patch needs improvement:  0
Easy pickings:  0 |UI/UX:  0
--+

Comment (by David Sanders):

 Hi Lufafa,

 It'd be best to check that Eric hasn't already started… (please read the
 comments above).

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018a6aa4498c-42cba4d1-c948-41ea-ad49-12b70c4a82c6-00%40eu-central-1.amazonses.com.

Re: [Django] #34808: Some aggregation functions may return None; this isn't well documented

[Django]

#34808: Some aggregation functions may return None; this isn't well documented
--+
 Reporter:  Eric Baumgartner  |Owner:  nobody
 Type:  Cleanup/optimization  |   Status:  new
Component:  Documentation |  Version:  4.2
 Severity:  Normal|   Resolution:
 Keywords:| Triage Stage:  Accepted
Has patch:  0 |  Needs documentation:  0
  Needs tests:  0 |  Patch needs improvement:  0
Easy pickings:  0 |UI/UX:  0
--+

Comment (by Lufafa Joshua):

 Hey Simon, i would love to help with this ticket. some guidance along the
 way will be helpful. Thanks

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018a6a9da16d-03125d16-ae92-45d7-8728-0c555d74eef4-00%40eu-central-1.amazonses.com.

Re: [Django] #34808: Some aggregation functions may return None; this isn't well documented

[Django]

#34808: Some aggregation functions may return None; this isn't well documented
--+
 Reporter:  Eric Baumgartner  |Owner:  nobody
 Type:  Cleanup/optimization  |   Status:  new
Component:  Documentation |  Version:  4.2
 Severity:  Normal|   Resolution:
 Keywords:| Triage Stage:  Accepted
Has patch:  0 |  Needs documentation:  0
  Needs tests:  0 |  Patch needs improvement:  0
Easy pickings:  0 |UI/UX:  0
--+

Comment (by David Sanders):

 Hi Eric,

 Thanks for putting your hand up to take a shot!  Writing docs isn't too
 hard, basically need to check out the repo and update the relevant file
 under docs/. It's all in Sphinx and each file corresponds to a page in the
 docs (the file path corresponds to the URL path, conveniently enough). You
 then run `make html` and point your browser to the generated html.

 See https://docs.djangoproject.com/en/dev/internals/contributing/writing-
 documentation/  Carlton also mentioned that there was some effort to try
 aligning the docs to: https://diataxis.fr/

 Once your happy with how it looks it's simply a matter of creating a pull
 request and referencing this ticket. (see commit log for message format)

 Re your points:

  - It's not just empty querysets that produce `None`. Empty groups in a
 non-empty queryset will do the same thing.
  - A section on handling `None` using `default` is a good idea 👍
 Explaining why it returns `None` by default is also a good idea.
  - I'm not sure whether it's worth going through every example to
 explicitly handle defaults, the dedicated section may be enough, Nessita &
 Felix may make a call on this.

 Good luck!

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018a59f74c68-71e3a7b0-cbd6-4757-ad5f-8a99ead22e41-00%40eu-central-1.amazonses.com.

Re: [Django] #34808: Some aggregation functions may return None; this isn't well documented

[Django]

#34808: Some aggregation functions may return None; this isn't well documented
--+
 Reporter:  Eric Baumgartner  |Owner:  nobody
 Type:  Cleanup/optimization  |   Status:  new
Component:  Documentation |  Version:  4.2
 Severity:  Normal|   Resolution:
 Keywords:| Triage Stage:  Accepted
Has patch:  0 |  Needs documentation:  0
  Needs tests:  0 |  Patch needs improvement:  0
Easy pickings:  0 |UI/UX:  0
--+

Comment (by Eric Baumgartner):

 Ah, thanks, I completely missed that `default` was an option. (In 4.0, at
 least -- I"m still working with some 3.2 projects.)

 I'm happy to take a shot at some documentation changes. I haven't
 contributed to Django before, so if you have any pointers for newbies I'd
 appreciate it.

 Now that I've reviewed #10929 (and I know what to look for) I think
 coverage is better than I'd originally thought. I think the main things
 I'd address include:

 - Reviewing all example code across pages that uses `.aggregate(...)` and
 adding an explicit `default` param or `None` handling when appropriate. I
 think this is important because many people grab the sample code and don't
 read additional documentation.
 - On the aggregation topic page specifically, add a new section ("Handling
 empty querysets"?) that discusses `default` and explains that it uses
 Coalesce under the hood.
 - Add something to the documentation of `Aggregation.default` on the
 querysets page mentioning that it uses Coalesce under the hood.

 Does that scope make sense?

 I think mentioning Coalesce is helpful because it provides a hint for
 pre-4 users who can't use `default`. Is that sort of thing OK? I don't
 know what the project's position is on mentioning stuff that only applies
 to older versions.

 Somewhat related, should the documentation for `default` have a "New in
 4.0" flag? Or are those flags reserved for just changed in the last
 release or two?

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018a56b633d4-ae693958-e2de-4833-ba5e-287ff396-00%40eu-central-1.amazonses.com.

Re: [Django] #34808: Some aggregation functions may return None; this isn't well documented

[Django]

#34808: Some aggregation functions may return None; this isn't well documented
--+
 Reporter:  Eric Baumgartner  |Owner:  nobody
 Type:  Cleanup/optimization  |   Status:  new
Component:  Documentation |  Version:  4.2
 Severity:  Normal|   Resolution:
 Keywords:| Triage Stage:  Accepted
Has patch:  0 |  Needs documentation:  0
  Needs tests:  0 |  Patch needs improvement:  0
Easy pickings:  0 |UI/UX:  0
--+
Changes (by Simon Charette):

 * cc: Nick Pope (added)
 * stage:  Unreviewed => Accepted


Comment:

 I guess we should augment the
 [https://docs.djangoproject.com/en/4.2/topics/db/aggregation/ aggregation
 topic] to more clearly highlight the usage of the `Aggregate.default`
 option added in 4.0 (see #10929) which was meant to be address this exact
 issue by providing [https://github.com/django/django/pull/14026/files
 #diff-c7d87b6e6b973bf87e5685442a6c1da9531c24bcaa86fe158c40ea0276986f32R68
 a shorthand that avoids wrapping every aggregate] in `Coalesce`
 (`Aggregate(default=42)` -> `Coalesce(Aggregate(), 42)`.

 Would you be interested in providing documentation adjustments Eric?

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018a4e6cdf55-51430707-ec72-470a-ad7e-c41cfb0da5a5-00%40eu-central-1.amazonses.com.

[Django] #34808: Some aggregation functions may return None; this isn't well documented

[Django]

#34808: Some aggregation functions may return None; this isn't well documented
+
   Reporter:  Eric Baumgartner  |  Owner:  nobody
   Type:  Cleanup/optimization  | Status:  new
  Component:  Documentation |Version:  4.2
   Severity:  Normal|   Keywords:
   Triage Stage:  Unreviewed|  Has patch:  0
Needs documentation:  0 |Needs tests:  0
Patch needs improvement:  0 |  Easy pickings:  0
  UI/UX:  0 |
+
 Calls like Book.objects.filter(...).aggregate(Sum("pages")) may return
 None if the queryset is empty.

 This isn't well documented and I think many of the examples in the
 documentation can lead to folks writing unsafe code that works most of the
 time, but can fail for empty queries.

 For example (using models from
 https://docs.djangoproject.com/en/4.2/topics/db/aggregation/):

 {{{#!python
 obscure_publisher = ...
 q =
 Book.objects.filter(publisher=obscure_publisher).Aggregate(Sum("pages"))
 total_pages = q["pages"]  # None if the publisher hasn't published any
 books!

 # Silly example, but will raise a TypeError
 kilopages = total_pages / 1000
 }}}

 From what I've seen online, I think the safer approach is to use Coalesce.

 {{{#!python
 obscure_publisher = ...
 q =
 
Book.objects.filter(publisher=obscure_publisher).Aggregate(Coalesce(Sum("pages"),
 0)
 total_pages = q["pages"]  # Safe; using coalesce guarantees an int.
 }}}

 The only reference I've found to using Coalesce in this way appears to be
 a usage example at https://docs.djangoproject.com/en/4.2/ref/models
 /database-functions/#coalesce. Coalesce is not mentioned at all on the
 main aggregation page.

 I think the documentation could be improved in a few ways.

 On https://docs.djangoproject.com/en/4.2/topics/db/aggregation/:

 - The code examples could consider adding Coalesce, or at least adding a
 comment at the top of the cheat sheet section mentioning that the examples
 do not include error checking and that using Coalesce is best practice.

 For aggregation functions mentioned in
 https://docs.djangoproject.com/en/4.2/ref/models/querysets/:

 - Add something to the documentation of the return type for functions used
 by aggregate() that mentions the empty queryset case if the result type
 differs for empty queries.
 - For example, Count() safely returns an int even with empty querysets,
 but Sum, Avg, Min, and Max do not.
 - Might also be worth mentioning that one can use Coalesce to avoid having
 to deal with None values.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/0107018a4e32cbcc-acf7f296-2855-4845-a4fb-6b3d952f13ad-00%40eu-central-1.amazonses.com.