Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#24656 closed Cleanup/optimization (fixed)

Query Expressions page do not always show imports

Reported by: Daniel Greenfeld Owned by: Nicolas Noé
Component: Documentation Version: 1.8
Severity: Normal Keywords:
Cc: josh.smeaton@…, nicolas@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

https://docs.djangoproject.com/en/1.8/ref/models/expressions/

Issues:

  • Import of the F object does not occur until the third example.
  • Func, Length, Aggregate, Value, ExpressionWrapper, and Count are never imported
  • Expression in the third-to-last example

From where are these objects to be imported?

Attachments (1)

django_patch_24656.diff (2.6 KB) - added by Nicolas Noé 4 years ago.

Download all attachments as: .zip

Change History (12)

comment:1 Changed 4 years ago by Josh Smeaton

Are you concerned that the examples do not show imports, or that the import location is not obvious when looking at the specific expressions?

If the first, that's understandable and could be made more clear.

If the second, each class is tagged with the import location, such as Length: https://docs.djangoproject.com/en/1.8/ref/models/database-functions/#django.db.models.functions.Length

This is somewhat consistent with other areas of the docs such as https://docs.djangoproject.com/en/1.8/ref/models/fields/#django.db.models.AutoField with the primary difference being that there is a specific note at the top that calls out alternate import locations: https://docs.djangoproject.com/en/1.8/ref/models/fields/#module-django.db.models.fields

Do we need to prominently show import locations (modules) for reference documentation rather than having it partially hidden within #anchors? The information exists in the raw docs, perhaps we could surface that somehow.

comment:2 Changed 4 years ago by Josh Smeaton

Cc: josh.smeaton@… added

comment:3 Changed 4 years ago by Tim Graham

Easy pickings: set
Triage Stage: UnreviewedAccepted
Type: UncategorizedCleanup/optimization

Examples not showing imports seems to be the complaint as far as I can tell. We could add a note at the top about expressions being available from django.db.models.

See also #15396.

comment:4 Changed 4 years ago by Daniel Greenfeld

Tim explains the issue better than I can. ;)

Yes, a note at the top of the page saying all expressions are available from django.db.models would be perfect.

For reference, I'm of the somewhat well-known firm belief that all code examples in documentation should either include the imports of framework objects, or mention their import path prominently in the documentation. Also, no offense, but I also think linking to the source code is rather unfair to beginners, being just a step above telling them to 'read the source'.

comment:5 Changed 4 years ago by Josh Smeaton

Fully agree that examples should have imports shown and that the import paths should be clear at either the top of the file or inline for each type.

Regarding your linking to the source comment, are you referencing the [source] annotations in some of the documentation? If so, those links should only ever be supplementary for the curious, and absolutely not required for complete understanding. I would disagree with the suggestion that they should be removed if that's the suggestion being made. It's entirely possible that I've misunderstood though.

If you have any other suggestions for improving the expressions documentation, please raise them. Writing documentation for your own code can be tricky, because you're not sure what is too little or too much information sometimes.

comment:6 Changed 4 years ago by Daniel Greenfeld

Apologies for not being clear on the source code issue. I'll try again. :)

  • Better descriptions are infinitely better than just providing links to source code.
  • On the other hand, linking to the source code is AWESOME. Keep it in! More like this please!

comment:7 Changed 4 years ago by Nicolas Noé

Owner: changed from nobody to Nicolas Noé
Status: newassigned

Changed 4 years ago by Nicolas Noé

Attachment: django_patch_24656.diff added

comment:8 Changed 4 years ago by Nicolas Noé

Cc: nicolas@… added
Has patch: set

comment:9 Changed 4 years ago by Nicolas Noé

I just submitted a patch that:

  • Adds all the required imports in the different exemples (except if they are one/two liners AND they're not the first example using the said expression in the page).
  • Adds an explicative block at the beginning of the "Built-in Expressions", stating that those expressions can indeed be imported from django.db.models as suggested by timgraham.

Hope that helps, that's almost my first contribution.

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

Resolution: fixed
Status: assignedclosed

In 3768236:

Fixed #24656 -- Added missing imports to query expressions doc.

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

In 61e902c4:

[1.8.x] Fixed #24656 -- Added missing imports to query expressions doc.

Backport of 37682368a604e08f3135375c85529e566492a352 from master

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