ModelForm: assert given instance is instance of model

[Thomas Güttler]

Hi,

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

[Thomas Güttler]

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

[Bastian Kleineidam <calvin@…>]

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.

[Malcolm Tredinnick]

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.

[Aymeric Augustin]

Change UI/UX from NULL to False.

[Aymeric Augustin]

Change Easy pickings from NULL to False.

[Selwin Ong]

I opened a pull request on Github to fix this issue here: ​https://github.com/django/django/pull/296 . Test included.

[Baptiste Mispelon]

This is technically backwards-incompatible, so a mention in the release notes is warranted I think.

Also, I'd like to see some more tests involving abstract models, inherited models or proxy ones, just to be sure we're not breaking any valid use-cases.

Thanks.

[Thomas Güttler]

I was the author of this ticket five years ago. Now I think different about this: if type checking at runtime is done here, then there are some hundred other places where it could be done. This would sometimes help programmers new to django and python, ..... but it would slow down the execution time and make the code less clean. I think this ticket can be closed (without a modification to django), but since there where contributions by other people, I will leave this decision to you.

[Jacob]

Yeah, I agree this can be closed wontfix: typechecking isn't particularly Pythonic, and I can think of a few edge-cases where duck-typing a model might be useful.