Django

Code

Ticket #6702 (new)

Opened 9 months ago

Last modified 1 month ago

ModelForm: assert given instance is instance of model

Reported by: guettli Assigned to: nobody
Milestone: Component: Forms
Version: SVN Keywords:
Cc: hv@tbz-pariv.de Triage Stage: Accepted
Has patch: 1 Needs documentation: 0
Needs tests: 1 Patch needs improvement: 0

Description

Hi,

this assert statement can help to find bugs early. Patch attached.

Attachments

newforms_models_assert_isinstance.diff (0.6 kB) - added by guettli on 03/03/08 06:08:14.

Change History

03/03/08 06:08:14 changed by guettli

  • attachment newforms_models_assert_isinstance.diff added.

03/03/08 06:10:18 changed by guettli

  • needs_better_patch changed.
  • needs_tests changed.
  • needs_docs changed.

BTW, all unittests pass 

modw@berry:~/_djangosvn/trunk/tests> ./runtests.py  forms  2> o-old
... uncomment assert statement.
modw@berry:~/_djangosvn/trunk/tests> ./runtests.py  forms  2> o-new
diff o-old o-newmodw@berry:~/_djangosvn/trunk/tests> diff o-old o-new
88c88
< Ran 28 tests in 1.649s
---
> Ran 28 tests in 0.936s

03/03/08 23:27:03 changed by Bastian Kleineidam <calvin@debian.org>

I don't like that asserts are removed when run with python -O. If a model instance is really required at this point, it would IMO be better to raise an exception.

03/04/08 08:23:58 changed by mtredinnick

  • stage changed from Unreviewed to Accepted.

An assert is fine here. Asserts are the right control structure for checking that the programmer did what they were meant to do in any case, since it's not user-supplied data (which would require a non-vanishing error). Let's remember that this is Python: ask forgiveness, not permission and all that. Paranoid pre-execution type checking isn't the norm.

10/28/08 07:21:43 changed by guettli

  • needs_tests set to 1.

Add/Change #6702 (ModelForm: assert given instance is instance of model)




Change Properties
Action