[Django] #25478: 1.8 migrating doc code sample needs parens

14 views
Skip to first unread message

Django

unread,
Sep 28, 2015, 12:06:28 AM9/28/15
to django-...@googlegroups.com
#25478: 1.8 migrating doc code sample needs parens
-------------------------------+--------------------
Reporter: nedbatchelder | Owner: nobody
Type: Uncategorized | Status: new
Component: Documentation | Version: 1.8
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------
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)
)
]
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/25478>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Sep 28, 2015, 2:55:42 AM9/28/15
to django-...@googlegroups.com
#25478: 1.8 migrating doc code sample needs parens
-------------------------------+--------------------------------------
Reporter: nedbatchelder | Owner: nobody
Type: Uncategorized | Status: closed
Component: Documentation | Version: 1.8
Severity: Normal | Resolution: worksforme
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------------------------
Changes (by claudep):

* status: new => closed
* needs_better_patch: => 0
* resolution: => worksforme
* needs_tests: => 0
* needs_docs: => 0


Comment:

Just tried an expression like this in a shell and apparently the parens
are not mandatory in this case.

{{{
>>> lst = [1, 2, 3, 'a']
>>> print([
... item
... for item in lst
... if item > 0
... and item != 'a'
... ])
[1, 2, 3]
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/25478#comment:1>

Django

unread,
Sep 28, 2015, 6:43:03 AM9/28/15
to django-...@googlegroups.com
#25478: 1.8 migrating doc code sample needs parens
-------------------------------+--------------------------------------

Reporter: nedbatchelder | Owner: nobody
Type: Uncategorized | Status: new
Component: Documentation | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed

Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------------------------
Changes (by nedbatchelder):

* status: closed => new
* resolution: worksforme =>


Comment:

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.

--
Ticket URL: <https://code.djangoproject.com/ticket/25478#comment:2>

Django

unread,
Sep 28, 2015, 8:03:47 AM9/28/15
to django-...@googlegroups.com
#25478: 1.8 migrating doc code sample needs parens
-------------------------------+------------------------------------
Reporter: nedbatchelder | Owner: nobody
Type: Bug | Status: new
Component: Documentation | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
Changes (by claudep):

* cc: pirosb3@… (added)
* type: Uncategorized => Bug
* stage: Unreviewed => Accepted


Comment:

Oh, sorry for misunderstanding! I think you are right.

--
Ticket URL: <https://code.djangoproject.com/ticket/25478#comment:3>

Django

unread,
Sep 28, 2015, 11:21:45 AM9/28/15
to django-...@googlegroups.com
#25478: 1.8 migrating doc code sample needs parens
-------------------------------+------------------------------------
Reporter: nedbatchelder | Owner: nobody
Type: Bug | Status: new
Component: Documentation | Version: 1.8

Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------

Comment (by timgraham):

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?

--
Ticket URL: <https://code.djangoproject.com/ticket/25478#comment:4>

Django

unread,
Sep 28, 2015, 11:54:12 AM9/28/15
to django-...@googlegroups.com
#25478: 1.8 migrating doc code sample needs parens
-------------------------------+------------------------------------
Reporter: nedbatchelder | Owner: nobody
Type: Bug | Status: new
Component: Documentation | Version: 1.8

Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------

Comment (by PirosB3):

It looks correct to me too. To make sure we could also write an automated
test for it if there isn't one already.

--
Ticket URL: <https://code.djangoproject.com/ticket/25478#comment:5>

Django

unread,
Sep 28, 2015, 1:23:58 PM9/28/15
to django-...@googlegroups.com
#25478: 1.8 migrating doc code sample needs parens
-------------------------------+--------------------------------------
Reporter: nedbatchelder | Owner: nobody
Type: Bug | Status: closed
Component: Documentation | Version: 1.8
Severity: Normal | Resolution: worksforme

Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------------------------
Changes (by timgraham):

* status: new => closed

* resolution: => worksforme


--
Ticket URL: <https://code.djangoproject.com/ticket/25478#comment:6>

Reply all
Reply to author
Forward
0 new messages