Opened 8 years ago

Closed 8 years ago

#6214 closed (fixed)

Small cleanup in models.base

Reported by: i_i Owned by: nobody
Component: Core (Other) Version: master
Severity: Keywords:
Cc: Triage Stage: Design decision needed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

fixed # TODO: Checking for the presence of '_meta' is hackish.

Attachments (4)

models.base.diff (528 bytes) - added by i_i 8 years ago.
fixes the previous patch
optimized.models.base.diff (1.1 KB) - added by i_i 8 years ago.
small optimization of previous patch
evenmoreoptimized.models.base.diff (4.6 KB) - added by i_i 8 years ago.
even more optimisations
models.base-11-01-2008.diff (4.6 KB) - added by i_i 8 years ago.
filter(lambda b...) changed to list comprehension

Download all attachments as: .zip

Change History (12)

comment:1 Changed 8 years ago by i_i

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Since each base can only be a Model subclass it always has the same metaclass and therefore always has a _meta attribute except the Model class itself.

comment:2 Changed 8 years ago by SmileyChris

  • Triage Stage changed from Unreviewed to Design decision needed

Push to a design decision, since this sounds about right but such a core change needs a bit of thinking.

comment:3 Changed 8 years ago by i_i

I can also suggest the renaming of the first argument of ModelBase.__new__ from cls to metacls for clarity, because it really is a ModelBase metaclass. It can be a source of confusion in future development and for people trying to understand what's happening since it's not clear from the code that cls here is NOT a newly created Model or Model-based class.

Oh, and ModelBase can be renamed to something like _ModelMetaclass because it is not a BASE class for Model it is a CLASS of Model class. And with first underscore if it is not intended for importing or customization.

comment:4 Changed 8 years ago by i_i

And, if we are talking about the design decision, I would also note that classmethods of Model (add_to_class and _prepare) can be safely moved to its metaclass as normal (not class) methods. It has the advantage of separating the logic of Model classes and Model instances.

comment:5 Changed 8 years ago by SmileyChris

Perhaps you could try to get a discussion going in the django-dev group.

Changed 8 years ago by i_i

fixes the previous patch

comment:6 follow-up: Changed 8 years ago by i_i

I mistakenly thought that only Model subclasses can be in bases. New patch fixes this.

Changed 8 years ago by i_i

small optimization of previous patch

Changed 8 years ago by i_i

even more optimisations

Changed 8 years ago by i_i

filter(lambda b...) changed to list comprehension

comment:8 Changed 8 years ago by mtredinnick

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

(In [7098]) Fixed #6214 -- Added some small optimisations to model class initialisation.
Thanks, Ivan Illarionov.

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