Future-proofing django.template (and elsewhere) against circular imports

4 views
Skip to first unread message

Tom Tobin

unread,
Feb 1, 2008, 1:47:46 PM2/1/08
to django-d...@googlegroups.com
I'm in the midst of resurrecting my "taghelpers" code that makes
writing complex template tags much easier; as with all my patches, I'm
using Mercurial to track Django locally while I work on them. Thing
is, I've moved nearly everything out of django.template.__init__ into
a django.template.main module in order to avoid some circular imports
... and it's nigh-impossible to automatically track upstream changes
to django.template.__init__ after that point.

That said, I'd like to propose that the majority of the contents of
django.template.__init__ be preemptively moved to
django.template.main, and imported back into __init__ as necessary.
Furthermore, I'd like to see all use of __init__ modules for holding
more than "moment-of-import" code to cease, for reasons of avoiding
said nasty circular imports.

Marty Alchin

unread,
Feb 1, 2008, 1:58:19 PM2/1/08
to django-d...@googlegroups.com
On Feb 1, 2008 1:47 PM, Tom Tobin <kor...@korpios.com> wrote:
> That said, I'd like to propose that the majority of the contents of
> django.template.__init__ be preemptively moved to
> django.template.main, and imported back into __init__ as necessary.
> Furthermore, I'd like to see all use of __init__ modules for holding
> more than "moment-of-import" code to cease, for reasons of avoiding
> said nasty circular imports.

And in some cases, it can be valuable from a pure maintainability
standpoint. django.db.models.fields.__init__, for instance, covers
many varied cases, and I've already moved the file-related code out
into a files.py as part of my work with #5361, just so I could manage
it more easily. But then, when there was a ModelForm ticket was
solved, in involved a change to FileField, which took entirely too
much digging to discover.

All in all, I'm +1 on this, but I don't know if it needs to be an
absolute, across-the-board mandate. For instance, django.http.__init__
could be split into django.http.request and django.http.response, but
I'm not really sure it needs to be done for that case.

-Gul

Malcolm Tredinnick

unread,
Feb 1, 2008, 9:29:06 PM2/1/08
to django-d...@googlegroups.com

I'm probably -0 on this. It's a little unnecessary code churn and being
able to track what lines changed when is unbelievable handy, so code
shuffling should be resisted.

Obviously some utility methods might need to be moved (I guess
standalone argument parsing functions are probably a case you hit), but
the overall template lexer and parser shouldn't be introducing circular
dependencies and really is the primary entry point.

I'd prefer to address specifics here and fix the particular
implementation oddities that cause problems than "let's move the whole
thing" at this point in our development. Working out "what were they
thinking/smoking when..." is too useful.

Malcolm

--
Atheism is a non-prophet organization.
http://www.pointy-stick.com/blog/

Tom Tobin

unread,
Feb 1, 2008, 9:40:24 PM2/1/08
to django-d...@googlegroups.com
On 2/1/08, Malcolm Tredinnick <mal...@pointy-stick.com> wrote:
> I'm probably -0 on this. It's a little unnecessary code churn and being
> able to track what lines changed when is unbelievable handy, so code
> shuffling should be resisted.

Copying in subversion retains history; why would we lose anything if
we created the new module via "svn cp" before further tweaking?

> Obviously some utility methods might need to be moved (I guess
> standalone argument parsing functions are probably a case you hit), but
> the overall template lexer and parser shouldn't be introducing circular
> dependencies and really is the primary entry point.

It's going to take a couple of days before I remember what blew up on
me; since I've been a naughty boy and haven't been merging my local
repository to trunk, I'm *way* off base at the moment (and stuck
gradually walking towards HEAD, fixing my patch in increments; this is
where git definitely has an advantage over hg, but I'm using the
latter).

> I'd prefer to address specifics here and fix the particular
> implementation oddities that cause problems than "let's move the whole
> thing" at this point in our development. Working out "what were they
> thinking/smoking when..." is too useful.

I agree with your reasoning, but I'm not sure it applies ... unless
I'm just plain wrong about Subversion's history tracking, in which
case, feel free to smack me upside the head. ;-)

Malcolm Tredinnick

unread,
Feb 1, 2008, 11:55:26 PM2/1/08
to django-d...@googlegroups.com

On Fri, 2008-02-01 at 20:40 -0600, Tom Tobin wrote:
> On 2/1/08, Malcolm Tredinnick <mal...@pointy-stick.com> wrote:
> > I'm probably -0 on this. It's a little unnecessary code churn and being
> > able to track what lines changed when is unbelievable handy, so code
> > shuffling should be resisted.
>
> Copying in subversion retains history; why would we lose anything if
> we created the new module via "svn cp" before further tweaking?

Yeha, you're right. I was having some kind of Saturday morning brain
failure. I have no real problem with just doing a mass copy except that
it doesn't really work out what the real problem is (i.e. why are their
circular import issues) and shouldn't be necessary.

Stuff intended for reuse inside a particular package doesn't really
belong in __init__ if the package contains multiple non-trivial files
(e.g. argument lexing and parsing), so no argument about moving those
bits out.

So I'm still -0 for lack of cause. Obviously +1 on moving any bits that
are necessary.

Malcolm

--
If it walks out of your refrigerator, LET IT GO!!
http://www.pointy-stick.com/blog/

Tom Tobin

unread,
Feb 2, 2008, 12:27:15 AM2/2/08
to django-d...@googlegroups.com
On 2/1/08, Malcolm Tredinnick <mal...@pointy-stick.com> wrote:
> Stuff intended for reuse inside a particular package doesn't really
> belong in __init__ if the package contains multiple non-trivial files
> (e.g. argument lexing and parsing), so no argument about moving those
> bits out.
>
> So I'm still -0 for lack of cause. Obviously +1 on moving any bits that
> are necessary.

The reason I'm concerned about code inside __init__ files is that it
increases the overall likelihood of a circular import issue, as well
as the possibility of other unintended side effects. Since __init__
gets run even if an import statement "merely passes through" to import
code from a submodule of the package, I'd prefer to see __init__ only
contain code that either 1) really should be getting run upon *any*
import of any part of the package, or 2) imports from subpackages for
namespacing purposes.

I doubt I'm being very convincing; I really need to throw together a
series of concrete examples of __init__ bugs for "show and tell"
before I revisit this. As for the specific code I could benefit from
having moved out right now, I'll know more tomorrow. (And now in
CST-land, I need sleep; the hours of my new job are hostile to the
night owl, and getting off schedule on weekend nights makes for a
cranky Monday!)

Justin Bronn

unread,
Feb 2, 2008, 12:50:21 PM2/2/08
to Django developers
> I'm *way* off base at the moment (and stuck gradually walking towards
> HEAD, fixing my patch in increments; this is where git definitely has an
> advantage over hg, but I'm using the latter).

I've found hg-svn to be particularly useful in helping me pull changes
from the various SVN Django branches -- if you aren't already using
it, it's definitely worth a try.

-Justin

Tom Tobin

unread,
Feb 2, 2008, 12:57:42 PM2/2/08
to django-d...@googlegroups.com

Nah, I mean something different — my patch is out of date, and if I
try to apply it against HEAD, it breaks ... and furthermore, it's hard
to immediately tell how to fix it. By gradually advancing towards
HEAD, and fixing smaller breakage as it comes up, I stand a better
chance of sanely figuring out how to bring it up to date. (I'm
already thinking up an extension that combines "hg bisect" with "mq",
hmm ....)

Adrian Holovaty

unread,
Feb 2, 2008, 7:10:55 PM2/2/08
to django-d...@googlegroups.com
On Feb 1, 2008 12:47 PM, Tom Tobin <kor...@korpios.com> wrote:
> That said, I'd like to propose that the majority of the contents of
> django.template.__init__ be preemptively moved to
> django.template.main, and imported back into __init__ as necessary.
> Furthermore, I'd like to see all use of __init__ modules for holding
> more than "moment-of-import" code to cease, for reasons of avoiding
> said nasty circular imports.

+1 on this. I actually started to do it myself about a month ago, but
it turned out to be more work than I could afford at the time. As I
was doing it, one thing that became clear very quickly is that quite a
few of the objects in the current django/template/__init__.py are
imported from other Django modules, so we'd have to be liberal in what
was imported into the new __init__.py. It would probably even be worth
doing "from django.template.main import *".

Adrian

--
Adrian Holovaty
holovaty.com | everyblock.com | djangoproject.com

Reply all
Reply to author
Forward
0 new messages