Code

Opened 9 years ago

Closed 9 years ago

Last modified 7 years ago

#239 closed defect (fixed)

Initializing a model instance should not require fields with blank

Reported by: maurycy Owned by: adrian
Component: Metasystem Version:
Severity: normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

   1.
      [maurycy@localhost ~]$ export DJANGO_SETTINGS_MODULE='model_test.settings.main'
   2.
      [maurycy@localhost ~]$ cat model_test/apps/test/models/test.py
   3.
      from django.core import meta
   4.
       
   5.
      class Test(meta.Model):
   6.
          fields = (
   7.
              meta.IntegerField('number', 'number', blank=True, default=666),
   8.
          )
   9.
          admin = meta.Admin()
  10.
      [maurycy@localhost ~]$ django-admin.py sqlall test
  11.
      BEGIN;
  12.
      CREATE TABLE test_tests (
  13.
          id integer NOT NULL PRIMARY KEY,
  14.
          number integer NOT NULL
  15.
      );
  16.
      INSERT INTO packages (label, name) VALUES ('test', 'test');
  17.
      INSERT INTO content_types (name, package, python_module_name) VALUES ('test', 'test', 'tests');
  18.
      INSERT INTO auth_permissions (name, package, codename) VALUES ('Can add test', 'test', 'add_test');
  19.
      INSERT INTO auth_permissions (name, package, codename) VALUES ('Can change test', 'test', 'change_test');
  20.
      INSERT INTO auth_permissions (name, package, codename) VALUES ('Can delete test', 'test', 'delete_test');
  21.
      COMMIT;
  22.
      [maurycy@localhost ~]$ django-admin.py install test
  23.
      [maurycy@localhost ~]$ python
  24.
      Python 2.4.1 (#1, May 16 2005, 15:19:29)
  25.
      [GCC 4.0.0 20050512 (Red Hat 4.0.0-5)] on linux2
  26.
      Type "help", "copyright", "credits" or "license" for more information.
  27.
      >>> from django.models.test import tests
  28.
      >>> test = tests.Test(id=None)
  29.
      >>> test.save()
  30.
      Traceback (most recent call last):
  31.
        File "<stdin>", line 1, in ?
  32.
        File "/usr/lib/python2.4/site-packages/django/core/meta.py", line 87, in _curried
  33.
          return args[0](*(args[1:]+moreargs), **dict(kwargs.items() + morekwargs.items()))
  34.
        File "/usr/lib/python2.4/site-packages/django/core/meta.py", line 737, in method_save
  35.
          f.pre_save(self, getattr(self, f.name), add)
  36.
      AttributeError: 'Test' object has no attribute 'number' 

Attachments (0)

Change History (10)

comment:1 Changed 9 years ago by maurycy

If it's not the bug, creating uninitialized blank fields with default should be better documented.

comment:2 Changed 9 years ago by hugo <gb@…>

Hmm. You have named your app "test". That's not a good idea with Python - test is a module in the standard library and depending on how imports are done you might stumble over this builtin "test" module. It's a quite usual people run into with python.

comment:3 Changed 9 years ago by jacob

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

This is a user error; you've got to fill in all the fields of the object if you're creating it by hand (note that manipulators handle this for you).

comment:4 Changed 9 years ago by maurycy

  • Resolution invalid deleted
  • Status changed from closed to reopened

I think that it should be done better.

I'm working on sessions subsystem and I have the following model:

    fields = (
        meta.CharField('name', 'name', maxlength=100, db_index=True, unique=True),
        meta.CharField('path', 'path', maxlength=60, default='/', blank=True, null=True),
        meta.CharField('domain', 'domain', maxlength=200, default=REGISTRATION_COOKIE_DOMAIN, blank=True),
        meta.IntegerField('max_age', 'max age in seconds', default=7200, blank=True),
        meta.BooleanField('is_secure', 'is secure', blank=True),
    )

Now, in the most cases no one will have to fill path, domain or max_age, because defaults are pretty good. Forcing users to fill them once more time, especially if default and blank are set, is unnecessary.

comment:5 Changed 9 years ago by adrian

  • priority changed from high to normal

comment:6 Changed 9 years ago by adrian

  • Status changed from reopened to new
  • Summary changed from AttributeError on fields with blank and default to Initializing a model instance should not require fields with blank=True or "default"

Ah, now I understand the issue. I agree that initializing a model instance shouldn't require fields with blank=True or "default" set. I'll work on the change.

comment:7 Changed 9 years ago by adrian

  • Status changed from new to assigned

comment:8 Changed 9 years ago by adrian

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

(In [360]) Fixed #239 and #107 -- Changed model init() to use Field.get_default() if the value wasn't explicitly passed as a keyword argument. That means setting 'id=None' is no longer necessary, and you can leave off fields if you want them to have default values set.

comment:9 Changed 9 years ago by maurycy

Great, thanks!

comment:10 Changed 8 years ago by Link

  • Summary changed from Initializing a model instance should not require fields with blank=True or "default" to Initializing a model instance should not require fields with blank

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.