Code

Opened 15 months ago

Closed 15 months ago

Last modified 5 weeks ago

#19689 closed Cleanup/optimization (fixed)

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

Reported by: charettes Owned by: charettes
Component: Database layer (models, ORM) Version: master
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())

Change History (15)

comment:1 Changed 15 months ago by charettes

  • Has patch set
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

All tests pass on Python 2.7.3 SQLite.

comment:2 Changed 15 months ago by claudep

  • Component changed from ORM aggregation to Database layer (models, ORM)
  • Triage Stage changed from Unreviewed to Ready for checkin

comment:3 Changed 15 months ago 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.

comment:4 Changed 15 months ago 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()?

comment:5 Changed 15 months ago by claudep

  • Triage Stage changed from Ready for checkin to Accepted

+1 for the renaming, and +1 for model_name

comment:6 Changed 15 months ago by charettes

  • Owner changed from nobody to charettes
  • Status changed from new to assigned

Alright, I'll tackle this.

comment:7 Changed 15 months ago by charettes

  • Needs documentation set
  • Needs tests set
  • Summary changed from Use `Options.module_name` instead of `object_name.lower()` to 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 Changed 15 months ago by charettes

  • 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 Changed 15 months ago by aaugustin

  • Triage Stage changed from Accepted to Ready 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 Changed 15 months ago by charettes

Alright, I'll update documentation and commit this.

comment:11 Changed 15 months ago by Simon Charette <charette.s@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

In ec469ade2b04b94bfeb59fb0fc7d9300470be615:

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

comment:12 Changed 5 weeks ago by Tim Graham <timograham@…>

In f6c1f05fbf8fdf980c91666d1c14990632e18503:

Removed Model._meta.module_name per deprecation timeline.

refs #19689.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.