[Django] #22819: Rename attribute `output_type` on the Query Expression API
Django
--------------------------------------+------------------------
Reporter: jorgecarleitao | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Uncategorized | Version: 1.7-beta-2
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------
Since the `output_type` used in the Query Expression API is becoming part
of the Public API in the 1.7,
I would like to propose to rename it before 1.7 is out. Here is why:
`output_type` is a property of classes that follow the Query Expression
API and is defined on any object that follows the Query Expression API as
{{{
@property
def output_type(self):
return IntegerField() # example, it only requires to be a subclass of
`Field`
}}}
However,
{{{
print(type(IntegerField())) # <class '__main__.IntegerField'>
print(type(IntegerField)) # <class 'type'>
}}}
IMHO, `output_type` can be misleadingly interpreted as being a `type`
rather than an `instance` of (a subclass of) a Field.
I imagine it would be very easy to someone write
{{{
@property
def output_type(self):
return CustomField
}}}
instead of
{{{
@property
def output_type(self):
return CustomField()
}}}
Given that some types that comply with the Query Expression API already
have the attribute `source` (Aggregate, Col, and GeoAggregate), one
possibility would be use just `output`.
--
Ticket URL: <https://code.djangoproject.com/ticket/22819>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: jorgecarleitao | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version:
Component: Database layer | 1.7-beta-2
(models, ORM) | Resolution:
Severity: Normal | Triage Stage:
Keywords: | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by jorgecarleitao):
* needs_better_patch: => 0
* component: Uncategorized => Database layer (models, ORM)
* needs_tests: => 0
* needs_docs: => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/22819#comment:1>
Django
Reporter: jorgecarleitao | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version:
Component: Database layer | 1.7-beta-2
(models, ORM) | Resolution:
Keywords: | Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timo):
* severity: Normal => Release blocker
Comment:
Bumping to release blocker so we make a decision before 1.7 is released.
--
Ticket URL: <https://code.djangoproject.com/ticket/22819#comment:2>
Django
Reporter: jorgecarleitao | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version:
Component: Database layer | 1.7-beta-2
(models, ORM) | Resolution:
Severity: Release blocker | Triage Stage:
Keywords: | Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by andrewgodwin):
Could we not enforce a check that it is a field instance and not a class?
That said, if the impact of renaming it is minimal, let's do it now.
--
Ticket URL: <https://code.djangoproject.com/ticket/22819#comment:3>
Django
Reporter: jorgecarleitao | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version:
Component: Database layer | 1.7-beta-2
(models, ORM) | Resolution:
Severity: Release blocker | Triage Stage:
Keywords: | Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by smeatonj):
If it were to be renamed, I vote for "output_field", which gives a little
more information about what it actually is - an instance of a model_field.
Documentation can describe what the current name is, but no such
documentation exists (yet) that is clear.
--
Ticket URL: <https://code.djangoproject.com/ticket/22819#comment:4>
Django
Reporter: jorgecarleitao | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version:
Component: Database layer | 1.7-beta-2
(models, ORM) | Resolution:
Severity: Release blocker | Triage Stage:
Keywords: | Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by akaariai):
The current name is something I just picked while hacking - there wasn't
that much though put into it. The reasoning is something along lines "what
data type the DB produces for the expression", hence output_type. Fields
happen to represent also database data types, so that is why field
instances are used.
If some better name can be found I don't have any objections against
changing the name.
--
Ticket URL: <https://code.djangoproject.com/ticket/22819#comment:5>
Django
Reporter: jorgecarleitao | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version:
Component: Database layer | 1.7-beta-2
(models, ORM) | Resolution:
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by charettes):
* stage: Unreviewed => Accepted
Comment:
Now we just have to agree on a name. @jorgecarleitao what do you think of
`output_field`?
--
Ticket URL: <https://code.djangoproject.com/ticket/22819#comment:6>
Django
Reporter: jorgecarleitao | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version:
Component: Database layer | 1.7-beta-2
(models, ORM) | Resolution:
Severity: Release blocker | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by jorgecarleitao):
I agree `output_field`, even more than `output` :)
--
Ticket URL: <https://code.djangoproject.com/ticket/22819#comment:7>
Django
Reporter: jorgecarleitao | Owner: timo
Type: | Status: assigned
Cleanup/optimization | Version:
Component: Database layer | 1.7-beta-2
(models, ORM) | Resolution:
Severity: Release blocker | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
* status: new => assigned
* owner: nobody => timo
--
Ticket URL: <https://code.djangoproject.com/ticket/22819#comment:8>
Django
Reporter: jorgecarleitao | Owner: timo
Cleanup/optimization | Version:
Component: Database layer | 1.7-beta-2
Severity: Release blocker | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"95cc0e15b45dd0986bdfd9fa3fedee4550c744c8"]:
{{{
#!CommitTicketReference repository=""
revision="95cc0e15b45dd0986bdfd9fa3fedee4550c744c8"
Fixed #22819 -- Renamed output_type -> output_field in query expression
API.
Thanks jorgecarleitao for the suggestion.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22819#comment:9>
Django
Reporter: jorgecarleitao | Owner: timo
Type: | Status: closed
Cleanup/optimization | Version:
Component: Database layer | 1.7-beta-2
(models, ORM) | Resolution: fixed
Severity: Release blocker | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"aa10f57d9460f121dea2ec2635e478ed02cc18b5"]:
{{{
#!CommitTicketReference repository=""
revision="aa10f57d9460f121dea2ec2635e478ed02cc18b5"
[1.7.x] Fixed #22819 -- Renamed output_type -> output_field in query
expression API.
Thanks jorgecarleitao for the suggestion.
Backport of 95cc0e15b4 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22819#comment:10>