Opened 18 years ago

Closed 18 years ago

Last modified 17 years ago

#1705 closed defect (fixed)

If you make an instance of a class without supplying kwargs the result is an instance with defects.

Reported by: emil.gruno@… Owned by: Adrian Holovaty
Component: Core (Other) Version:
Severity: normal 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

Intro

I am puzzled by the way one create an instance of a class, you have to
parse valid (memberdata) fieldname='foobar' to create the instance with the
memberdata attributes from self._meta.fields

Example;

The first example shows what i mean;

mysite/myapp/models.py

# -*- coding: cp1252 -*-
from django.db import models
class TestModel(models.Model):
    memberdata = models.CharField(maxlength=100)

>>> from mysite.myapp.models import TestModel
>>> t = TestModel()
>>> dir(t)

['AddManipulator', 'ChangeManipulator', 'DoesNotExist', '__class__',
'__delattr_
_', '__dict__', '__doc__', '__eq__', '__getattribute__', '__hash__',
'__init__',
 '__metaclass__', '__module__', '__ne__', '__new__', '__reduce__',
'__reduce_ex_
_', '__repr__', '__setattr__', '__str__', '__weakref__',
'_collect_sub_objects',
 '_default_manager', '_get_FIELD_display', '_get_FIELD_filename',
'_get_FIELD_he
ight', '_get_FIELD_size', '_get_FIELD_url', '_get_FIELD_width',
'_get_image_dime
nsions', '_get_next_or_previous_by_FIELD',
'_get_next_or_previous_in_order', '_g
et_pk_val', '_meta', '_prepare', '_save_FIELD_file',
'_set_related_many_to_many'
, 'add_to_class', 'delete', 'objects', 'save', 'validate']

id, memberdata etc.. is missing

>>> t = TestModel(memberdata='Hello World')
>>> dir(t)

['AddManipulator', 'ChangeManipulator', 'DoesNotExist', '__class__',
'__delattr_
_', '__dict__', '__doc__', '__eq__', '__getattribute__', '__hash__',
'__init__',
 '__metaclass__', '__module__', '__ne__', '__new__', '__reduce__',
'__reduce_ex_
_', '__repr__', '__setattr__', '__str__', '__weakref__',
'_collect_sub_objects',
 '_default_manager', '_get_FIELD_display', '_get_FIELD_filename',
'_get_FIELD_he
ight', '_get_FIELD_size', '_get_FIELD_url', '_get_FIELD_width',
'_get_image_dime
nsions', '_get_next_or_previous_by_FIELD',
'_get_next_or_previous_in_order', '_g
et_pk_val', '_meta', '_prepare', '_save_FIELD_file',
'_set_related_many_to_many'
, 'add_to_class', 'delete', 'id', 'memberdata', 'objects', 'save',
'validate']

if i had a member function like this:

def __repr__(self):
        return self.memberdata

And call it like this:

t = TestModel()
t + enter

This will fail to represent it self. =/ i think it should
not.

the following patch will fix that:

Index: magic-removal/django/db/models/base.py
===================================================================
--- magic-removal/django/db/models/base.py      (revision 2747)
+++ magic-removal/django/db/models/base.py      (working copy)
 -107,6 +107,9 @@
                     setattr(self, f.attname, val)
             if kwargs:
                 raise TypeError, "'%s' is an invalid keyword argument
for this function" % kwargs.keys()[0]
+        else:
+            for f in self._meta.fields:
+                setattr(self, f.attname, None)
         for i, arg in enumerate(args):
             setattr(self, self._meta.fields[i].attname, arg)
         dispatcher.send(signal=signals.post_init,
sender=self.__class__, instance=self)

Cheers,

Emil Gruno

Change History (3)

comment:1 by James Bennett, 18 years ago

So far as I know, that's deliberate; until you assign at least one actual field/value pair, there's little to no point in having an ID or many of the other attributes. For example, in your case where the repr returns the 'memberdata' field, it shouldn't work unless something's actually been assigned to 'memberdata'.

comment:2 by Luke Plant, 18 years ago

ubernostrum,

The problem is that setting a field/value after creating the instance doesn't fix things up. So for instance, if you have a model with field1, field2, and field3, this works

m = MyModel(field1='foo')
m.field2 = 'bar'
m.save() # field3 will have been set to a default.

But this doesn't:

m = MyModel()
m.field1 = 'foo'
m.field2 = 'bar'
m.save() # Raises an AttributeError

I've come across this lots of times (especially in my migration scripts and at the interactive prompt, and always just put up with it till now, but I would like to see it fixed. Another important use case would be:

m = MyModel()
fill_in_core_data(m)
m.save()

...
# MyExtendedModel has similar interface to MyModel
m2 = MyExtendedModel()
fill_in_core_data(m2)
m2.extra_field = 'foo'
m2.save()

There is an easier and more robust fix than the patch though -- just remove the if kwargs check.

comment:3 by Luke Plant, 18 years ago

Resolution: fixed
Status: newclosed

(In [2767]) magic-removal: Fixed #1705 - creating instances of models without supplying any keyword arguments.
(NB - diff is misleading - we need diff tools that support significant whitespace!)

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