[Django] #19689: Use `Options.module_name` instead of `object_name.lower()`
Django
--------------------------------------+--------------------
Reporter: charettes | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: ORM aggregation | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+--------------------
`django.db.options.Options` defines a `module_name =
self.object_name.lower()`
[https://github.com/django/django/blob/3647c0a49a2f4535b8a9aba40e662743e4d53e76/django/db/models/options.py#L80-L81
property] which should be used across code base instead of referring to
the lowercase of `object_name`.
Heres a `grep "object_name.lower()" django/ -R` of the code base.
{{{
django/views/generic/detail.py: return
obj._meta.object_name.lower()
django/views/generic/detail.py:
self.object._meta.object_name.lower(),
django/views/generic/detail.py:
self.model._meta.object_name.lower(),
django/views/generic/list.py: return '%s_list' %
object_list.model._meta.object_name.lower()
django/views/generic/list.py: names.append("%s/%s%s.html" %
(opts.app_label, opts.object_name.lower(), self.template_name_suffix))
django/db/models/options.py: self.module_name =
self.object_name.lower()
django/db/models/options.py: model_label = '%s.%s' %
(self.app_label, self.object_name.lower())
django/db/models/options.py: return 'add_%s' %
self.object_name.lower()
django/db/models/options.py: return 'change_%s' %
self.object_name.lower()
django/db/models/options.py: return 'delete_%s' %
self.object_name.lower()
django/db/models/fields/related.py: self.name = self.name or
(self.rel.to._meta.object_name.lower() + '_' + self.rel.to._meta.pk.name)
django/db/models/fields/related.py: return self.rel.related_name or
self.opts.object_name.lower()
django/db/models/fields/related.py: to_name =
to._meta.object_name.lower()
django/db/models/fields/related.py: from_ =
klass._meta.object_name.lower()
django/db/models/loading.py: model_name =
model._meta.object_name.lower()
django/db/models/related.py: self.var_name =
self.opts.object_name.lower()
django/db/models/related.py: return self.field.rel.related_name
or (self.opts.object_name.lower() + '_set')
django/db/models/related.py: return self.field.rel.related_name
or (self.opts.object_name.lower())
django/core/management/sql.py: sql_files = [os.path.join(app_dir,
"%s.%s.sql" % (opts.object_name.lower(), backend_name)),
django/core/management/sql.py: os.path.join(app_dir,
"%s.sql" % opts.object_name.lower())]
django/core/xheaders.py: response['X-Object-Type'] = "%s.%s" %
(model._meta.app_label, model._meta.object_name.lower())
django/contrib/admindocs/views.py: if m._meta.object_name.lower()
== model_name:
django/contrib/admin/options.py: "admin/%s/%s/change_form.html"
% (app_label, opts.object_name.lower()),
django/contrib/admin/options.py: 'admin/%s/%s/change_list.html'
% (app_label, opts.object_name.lower()),
django/contrib/admin/options.py:
"admin/%s/%s/delete_confirmation.html" % (app_label,
opts.object_name.lower()),
django/contrib/admin/options.py:
"admin/%s/%s/object_history.html" % (app_label, opts.object_name.lower()),
django/contrib/admin/widgets.py: info = (rel_to._meta.app_label,
rel_to._meta.object_name.lower())
django/contrib/admin/util.py:
opts.object_name.lower()),
django/contrib/admin/actions.py:
"admin/%s/%s/delete_selected_confirmation.html" % (app_label,
opts.object_name.lower()),
django/contrib/contenttypes/generic.py: opts.app_label,
opts.object_name.lower(),
django/contrib/contenttypes/generic.py: return
'-'.join((opts.app_label, opts.object_name.lower(),
django/contrib/contenttypes/management.py:
(model._meta.object_name.lower(), model)
django/contrib/contenttypes/models.py: key = (opts.app_label,
opts.object_name.lower())
django/contrib/contenttypes/models.py: model =
opts.object_name.lower(),
django/contrib/contenttypes/models.py:
needed_models.add(opts.object_name.lower())
django/contrib/contenttypes/models.py:
model=opts.object_name.lower(),
django/contrib/contenttypes/models.py: key =
(model._meta.app_label, model._meta.object_name.lower())
django/contrib/auth/management/__init__.py: return '%s_%s' % (action,
opts.object_name.lower())
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/19689>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: charettes | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version: master
Component: ORM aggregation | Resolution:
Severity: Normal | Triage Stage:
Keywords: | Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by charettes):
* needs_better_patch: => 0
* has_patch: 0 => 1
* needs_tests: => 0
* needs_docs: => 0
Comment:
All tests pass on Python 2.7.3 SQLite.
--
Ticket URL: <https://code.djangoproject.com/ticket/19689#comment:1>
Django
Reporter: charettes | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version: master
(models, ORM) | Triage Stage: Ready for
Severity: Normal | checkin
Keywords: | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by claudep):
* component: ORM aggregation => Database layer (models, ORM)
* stage: Unreviewed => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/19689#comment:2>
Django
Reporter: charettes | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version: master
Component: Database layer | Resolution:
(models, ORM) | Triage Stage: Ready for
Severity: Normal | checkin
Keywords: | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by aaugustin):
The patch looks good.
However I don't understand why the name `module_name` was chosen to
represent the lowercased model name. "Modules" are a well-defined concept
in Python, and this isn't a module. Could we take this opportunity to
replace it with a more meaningful name like `object_name_lower`?
Theoretically `Model._meta` is a private API but it's widely (ab)used. We
can keep a `module_name` property that raises a deprecation warning and
returns `object_name_lower`.
This refactoring needs to be done carefully because Django also uses
variables called `module_name` to store names of modules in some places.
--
Ticket URL: <https://code.djangoproject.com/ticket/19689#comment:3>
Django
Reporter: charettes | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version: master
Component: Database layer | Resolution:
(models, ORM) | Triage Stage: Ready for
Severity: Normal | checkin
Keywords: | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by charettes):
What about `model_name` instead of `object_name_lower`, seems more
meaningful to me and there's a couple of occurrence of it in the code base
that makes reference to it. e.g. `model_name =
model._meta.object_name.lower()`?
--
Ticket URL: <https://code.djangoproject.com/ticket/19689#comment:4>
Django
Reporter: charettes | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version: master
Component: Database layer | Resolution:
Severity: Normal | Needs documentation: 0
Keywords: | Patch needs improvement: 0
Has patch: 1 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by claudep):
* stage: Ready for checkin => Accepted
Comment:
+1 for the renaming, and +1 for `model_name`
--
Ticket URL: <https://code.djangoproject.com/ticket/19689#comment:5>
Django
Reporter: charettes | Owner: charettes
Type: | Status: assigned
Cleanup/optimization | Version: master
Component: Database layer | Resolution:
(models, ORM) | Triage Stage: Accepted
Severity: Normal | Needs documentation: 0
Keywords: | Patch needs improvement: 0
Has patch: 1 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
* status: new => assigned
* owner: nobody => charettes
Comment:
Alright, I'll tackle this.
--
Ticket URL: <https://code.djangoproject.com/ticket/19689#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: charettes | Owner: charettes
Type: | Status: assigned
Cleanup/optimization | Version: master
Component: Database layer | Resolution:
(models, ORM) | Triage Stage: Accepted
Keywords: | Patch needs improvement: 0
Has patch: 1 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by charettes):
* needs_docs: 0 => 1
* needs_tests: 0 => 1
Comment:
Added a patch that raises a warning when accessing `module_name` and
returns the newly introduced `model_name`.
Also made some cleanup in `django/contrib/auth/context_processors.py`
since it was manipulating `app_label`s using a variable/parameter named
`module_name`.
--
Ticket URL: <https://code.djangoproject.com/ticket/19689#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: charettes | Owner: charettes
Type: | Status: assigned
Cleanup/optimization | Version: master
Component: Database layer | Resolution:
(models, ORM) | Triage Stage: Accepted
Keywords: | Patch needs improvement: 0
Has patch: 1 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by charettes):
* needs_docs: 1 => 0
* needs_tests: 1 => 0
Comment:
Added a patch with release note and deprecation timeline entry. I'd
appreciate if someone could take a look at my wording.
All tests pass on Python 2.7.3 SQLite3.
--
Ticket URL: <https://code.djangoproject.com/ticket/19689#comment:8>
Django
-------------------------------------+-------------------------------------
Reporter: charettes | Owner: charettes
Type: | Status: assigned
Cleanup/optimization | Version: master
Component: Database layer | Resolution:
Severity: Normal | checkin
Keywords: | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by aaugustin):
* stage: Accepted => Ready for checkin
Comment:
Looks good. I'd suggest the following wording:
Deprecation timeline:
> `Model._meta.module_name` was renamed to `model_name`.
Release notes:
> `Model._meta.module_name` was renamed to `model_name`. Despite being a
private API, it will go through a regular deprecation path.
The release notes are for our users. The deprecation timeline is just a
list for advancing deprecations and usually more concise.
--
Ticket URL: <https://code.djangoproject.com/ticket/19689#comment:9>
Django
-------------------------------------+-------------------------------------
Reporter: charettes | Owner: charettes
Type: | Status: assigned
Cleanup/optimization | Version: master
Component: Database layer | Resolution:
(models, ORM) | Triage Stage: Ready for
Severity: Normal | checkin
Keywords: | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by charettes):
Alright, I'll update documentation and commit this.
--
Ticket URL: <https://code.djangoproject.com/ticket/19689#comment:10>
Django
-------------------------------------+-------------------------------------
Reporter: charettes | Owner: charettes
Cleanup/optimization | Version: master
Component: Database layer | Resolution: fixed
(models, ORM) | Triage Stage: Ready for
Severity: Normal | checkin
Keywords: | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"ec469ade2b04b94bfeb59fb0fc7d9300470be615"]:
{{{
#!CommitTicketReference repository=""
revision="ec469ade2b04b94bfeb59fb0fc7d9300470be615"
Fixed #19689 -- Renamed `Model._meta.module_name` to `model_name`.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/19689#comment:11>
Django
-------------------------------------+-------------------------------------
Reporter: charettes | Owner: charettes
Type: | Status: closed
Cleanup/optimization | Version: master
Component: Database layer | Resolution: fixed
(models, ORM) | Triage Stage: Ready for
Severity: Normal | checkin
Keywords: | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"f6c1f05fbf8fdf980c91666d1c14990632e18503"]:
{{{
#!CommitTicketReference repository=""
revision="f6c1f05fbf8fdf980c91666d1c14990632e18503"
Removed Model._meta.module_name per deprecation timeline.
refs #19689.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/19689#comment:12>