Django

Code

Ticket #1705 (closed: fixed)

Opened 2 years ago

Last modified 2 years ago

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

Reported by: emil.gruno@gmail.com Assigned to: adrian
Milestone: Component: Core framework
Version: Keywords:
Cc: Triage Stage: Unreviewed
Has patch: 0 Needs documentation: 0
Needs tests: 0 Patch needs improvement: 0

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

Attachments

Change History

04/27/06 12:04:51 changed by ubernostrum

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'.

04/27/06 13:25:36 changed by lukeplant

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.

04/28/06 12:59:38 changed by lukeplant

  • status changed from new to closed.
  • resolution set to fixed.

(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!)


Add/Change #1705 (If you make an instance of a class without supplying kwargs the result is an instance with defects.)




Change Properties
Action