Opened 18 years ago

Closed 18 years ago

Last modified 18 years ago

#1469 closed defect (wontfix)

Move django.forms to django.form.fields

Reported by: anonymous Owned by: Adrian Holovaty
Component: Validators Version: magic-removal
Severity: minor Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

django.forms is just the init.py file which contains all the code for the module.
By python "standards" the file is only used to initialize a module not to store the entire module in.
solution: move code into fields.py in the same directory.

Change History (5)

comment:1 by Jacob, 18 years ago

Resolution: wontfix
Status: newclosed

I'm not sure what standards you're referring to. If it's not broken...

comment:2 by gary.wilson@…, 18 years ago

Resolution: wontfix
Status: closedreopened

From http://docs.python.org/tut/node8.html:

The __init__.py files are required to make Python treat the directories as containing packages; this is done to prevent directories with a common name, such as "string", from unintentionally hiding valid modules that occur later on the module search path. In the simplest case, __init__.py can just be an empty file, but it can also execute initialization code for the package or set the __all__ variable, described later.

From http://twistedmatrix.com/projects/core/documentation/howto/policy/coding-standard.html#auto4:

__init__.py must never contain anything other than a docstring and (optionally) an __all__ attribute. Packages are not modules and should be treated differently. This rule may be broken to preserve backwards compatibility if a module is made into a nested package as part of a refactoring.


There does not seem to be much documentation on coding standards of __init__.py files. Their purpose is to turn a directory into a package and they are generally used for package documentation and initialization. For django.forms.__init__.py, I can see FORM_FIELD_ID_PREFIX and EmptyValue being initialization, and maybe even a few of the other classes; the FormField widgets are certainly not initialization.

Another point would be that except for the __init__.py file, the package is empty. If it were going to stay as is and not be broken down then it should be a module, not a package. I say that it should be broken down into submodules because ~1000 line files are not fun.

comment:3 by Malcolm Tredinnick <malcolm@…>, 18 years ago

The first example you quote is from the most introductory Python document there is -- designed for newcomers, so it doesn't go into details about all the possibilities. The second example is from another project (and just because they choose this interpretation does not mean it's universally "natural").

By way of counter-example, look at logging/__init__.py, hotshot/__init__.py and mail/__init__.py from your core Python distribution (and there are others, too). Lots of examples of non-trivial __init__.py usage there (maybe not as much as Django, but, again, that is just stylistic).

This is really just a stylistic thing and we could argue over those things all day without making real progres on improving Django. It comes down to preferring to open forms.py rather than forms/init.py (the latter does allow future expansion to sub-packages, by the way) in this case. I agree with Jacob: "not broken".

comment:4 by Jacob, 18 years ago

Resolution: wontfix
Status: reopenedclosed

What Malcolm said :)

comment:5 by anonymous, 18 years ago

That "most introductory Python document" happens to be copied verbatim from GvR's packages essay.

I would agree with you that logging does have a lot (much unnecessary IMO) in its __init__.py file, but it also does a lot of initialization. It sets up module data, default logging levels, threading and locking, the default formatter, an internal list of handlers to close upon shutdown, the root logger, log level convenience functions, and exit hooks.

It also makes things simple for the most common use case:

import logging
logging.error("There was an error.")

Well, it's not "just" a stylistic thing, __init__.py runs when a submodule/subpackage is imported; not a big deal here, though, since there aren't any. But in, say, /django/db/models/manipulators.py where we have

from django.db.models.fields.related import ManyToOne

not only will /django/db/models/fields/related.py get run, but /django/db/models/fields/__init__.py et al. get run also.

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