Opened 7 years ago
Last modified 7 years ago
#28454 closed Cleanup/optimization
Simplify use of Query.setup_joins() by returning a named tuple — at Version 1
| Reported by: | Matthew Wilkes | Owned by: | matthewwilkes |
|---|---|---|---|
| Component: | Database layer (models, ORM) | Version: | dev |
| Severity: | Normal | Keywords: | orm |
| Cc: | Triage Stage: | Accepted | |
| Has patch: | no | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description (last modified by )
This is split from review of #24747's PR, specifically https://github.com/django/django/pull/8528#discussion-diff-127578267R1463
@jarshwah
This is getting out of hand, especially considering callers ignoring so many of the return values. Does it make sense to create a type to model these values? At a minimum I think we should consider namedtuple.
@MatthewWilkes
No argument here, I got a bit of underscore blindness at times. Think that should be its own PR, so we don't have too much refactoring in with functionality changes?
@timgraham
Yes please
The Query.setup_joins() method currently returns a tuple of many pieces of data. #24747 adds another to that list. As many users of setup_joins() unpacked multiple variables to _ placeholders, it was decided to simplify usage and make it easier to add additional items to the tuple without changing the unpacking.
Change History (1)
comment:1 by , 7 years ago
| Component: | Uncategorized → Database layer (models, ORM) |
|---|---|
| Description: | modified (diff) |
| Summary: | Simplify use of setup_joins by returning a named tuple → Simplify use of Query.setup_joins() by returning a named tuple |
| Triage Stage: | Unreviewed → Accepted |