Opened 11 years ago

Closed 11 years ago

Last modified 10 years ago

#19689 closed Cleanup/optimization (fixed)

Deprecate `Options.module_name` in favor of `model_name`

Reported by: Simon Charette Owned by: Simon Charette
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

django.db.options.Options defines a module_name = self.object_name.lower() 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())

Attachments (3)

0001-Fixed-19686-Use-module_name-instead-of-object_name.l.patch (19.0 KB ) - added by Simon Charette 11 years ago.
0001-Fixed-19689-Deprecated-meta-s-module_name-in-favor-o.patch (41.8 KB ) - added by Simon Charette 11 years ago.
0001-Fixed-19689-Deprecated-model-meta-s-module_name-in-f.patch (43.0 KB ) - added by Simon Charette 11 years ago.
with doc

Download all attachments as: .zip

Change History (15)

comment:1 by Simon Charette, 11 years ago

Has patch: set

All tests pass on Python 2.7.3 SQLite.

comment:2 by Claude Paroz, 11 years ago

Component: ORM aggregationDatabase layer (models, ORM)
Triage Stage: UnreviewedReady for checkin

comment:3 by Aymeric Augustin, 11 years ago

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.

comment:4 by Simon Charette, 11 years ago

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()?

comment:5 by Claude Paroz, 11 years ago

Triage Stage: Ready for checkinAccepted

+1 for the renaming, and +1 for model_name

comment:6 by Simon Charette, 11 years ago

Owner: changed from nobody to Simon Charette
Status: newassigned

Alright, I'll tackle this.

comment:7 by Simon Charette, 11 years ago

Needs documentation: set
Needs tests: set
Summary: Use `Options.module_name` instead of `object_name.lower()`Deprecate `Options.module_name` in favor of `model_name`

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_labels using a variable/parameter named module_name.

comment:8 by Simon Charette, 11 years ago

Needs documentation: unset
Needs tests: unset

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.

comment:9 by Aymeric Augustin, 11 years ago

Triage Stage: AcceptedReady for checkin

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.

comment:10 by Simon Charette, 11 years ago

Alright, I'll update documentation and commit this.

comment:11 by Simon Charette <charette.s@…>, 11 years ago

Resolution: fixed
Status: assignedclosed

In ec469ade2b04b94bfeb59fb0fc7d9300470be615:

Fixed #19689 -- Renamed Model._meta.module_name to model_name.

comment:12 by Tim Graham <timograham@…>, 10 years ago

In f6c1f05fbf8fdf980c91666d1c14990632e18503:

Removed Model._meta.module_name per deprecation timeline.

refs #19689.

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