Opened 7 years ago
Closed 7 years ago
#28454 closed Cleanup/optimization (fixed)
Simplify use of Query.setup_joins() by returning a named tuple
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 (2)
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 |
comment:2 by , 7 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
In 32d1bf2: