Opened 9 years ago
Closed 9 years ago
#25478 closed Bug (worksforme)
1.8 migrating doc code sample needs parens
Reported by: | Ned Batchelder | Owned by: | nobody |
---|---|---|---|
Component: | Documentation | Version: | 1.8 |
Severity: | Normal | Keywords: | |
Cc: | pirosb3@… | Triage Stage: | Accepted |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
I think this sample from https://docs.djangoproject.com/en/1.8/ref/models/meta/#migrating-from-the-old-api needs parens to be correct. The docs have:
[ (f, f.model if f.model != MyModel else None) for f in MyModel._meta.get_fields() if not f.is_relation or f.one_to_one or (f.many_to_one and f.related_model) ]
but the condition seems wrong. Shouldn't it be:
[ (f, f.model if f.model != MyModel else None) for f in MyModel._meta.get_fields() if not (f.is_relation or f.one_to_one or (f.many_to_one and f.related_model) ) ]
Change History (6)
comment:1 by , 9 years ago
Resolution: | → worksforme |
---|---|
Status: | new → closed |
comment:2 by , 9 years ago
Resolution: | worksforme |
---|---|
Status: | closed → new |
I didn't mean that it was a syntax error. I meant that the logic was wrong. Without the parens, the condition is equivalent to:
(not f.is_relation) or (f.one_to_one) or (f.many_to_one and f.related_model)
Don't we actually want:
not (f.is_relation or f.one_to_one or (f.many_to_one and f.related_model))
That is, the field should be excluded if it's a relation or if it's one-to-one, or if it's both many-to-one and related.
comment:3 by , 9 years ago
Cc: | added |
---|---|
Triage Stage: | Unreviewed → Accepted |
Type: | Uncategorized → Bug |
Oh, sorry for misunderstanding! I think you are right.
comment:4 by , 9 years ago
It looks correct to me. The idea seems to be to exclude all relations except foreign keys and one-to-one fields. Here's an example from djangoproject.com where you can see those fields included in the old API:
>>> from accounts.models import Profile >>> Profile._meta.get_fields_with_model() [(<django.db.models.fields.AutoField: id>, None), (<django.db.models.fields.related.OneToOneField: user>, None), (<django.db.models.fields.CharField: name>, None)]
Can you provide an example discrepancy?
comment:5 by , 9 years ago
It looks correct to me too. To make sure we could also write an automated test for it if there isn't one already.
comment:6 by , 9 years ago
Resolution: | → worksforme |
---|---|
Status: | new → closed |
Just tried an expression like this in a shell and apparently the parens are not mandatory in this case.