Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#24665 closed Bug (fixed)

Model field relational attributes are None by default, not False

Reported by: Markus Holtermann Owned by: honya121
Component: Database layer (models, ORM) Version: 1.8
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The docs state that the relational attributes, such as many_to_many, one_to_many, etc should be False by default (https://docs.djangoproject.com/en/1.8/ref/models/fields/#attributes-for-fields-with-relations). In practice they are None though.

According to Daniel on IRC they should be False not None (i.e. the documentation is correct).

Change History (14)

comment:2 by pirosb3, 9 years ago

yoongkang, are you doing this?

comment:3 by Yoong Kang Lim, 9 years ago

pirosb3, sorry I think I must have mixed this up with a different issue. How do I assign it back to you?

comment:4 by honya121, 9 years ago

Owner: changed from pirosb3 to honya121
Status: newassigned

comment:5 by honya121, 9 years ago

It seems that the relational attributes are None only for non-relational fields (Field.is_relation=False). For relational fields these attributes are False or True. This is right behaviour according to the documentation (the values should only be meaningful if Field.is_relation=True), but it is possible, that I tested it badly. So there are 2 options:
a) keep default relational values in the non-relational fields None
b) set default relational values for ALL fields to False

For both options, it would be good to write what values are default even for fields with Field.is_relation=False, instead of writing that these fields will have unmeaningful values.

comment:6 by Markus Holtermann, 9 years ago

Hi honya121, in my opinion you should go forward with option b. I don't understand what you mean with your last sentence.

comment:7 by honya121, 9 years ago

Hi, ok, I'll do the b) option. The last sentence is about documentation. Sorry, I forgot to mention that. Anyway, in the documentation, there should be also mentioned, that the relational attributes will be False for non-relational fields.

comment:8 by Tim Graham, 9 years ago

Has patch: set

comment:9 by Tim Graham, 9 years ago

I'd like to better understand the rationale for this change. In my mind, None for non-relational fields seems like it may make debugging easier, e.g. if you find a None value, then you'll know you forgot to check is_relation.

comment:10 by Carl Meyer, 9 years ago

I agree with timgraham. It seems entirely sensible to me to have the relation-cardinality attributes be None (as in, neither True nor False because they are not even applicable in the first place) for non-relational fields. I don't see the rationale for changing them to False.

comment:11 by Markus Holtermann, 9 years ago

Either way makes sense in my eyes. E.g. "Am I a one_to_many field?" -- "No" --> False. In that case I think we still need to update the docs.

comment:12 by Tim Graham, 9 years ago

  • docs/ref/models/fields.txt

    diff --git a/docs/ref/models/fields.txt b/docs/ref/models/fields.txt
    index 01ff654..f8144c1 100644
    a b Attributes for fields with relations  
    18831883
    18841884These attributes are used to query for the cardinality and other details of a
    18851885relation. These attribute are present on all fields; however, they will only
    1886 have meaningful values if the field is a relation type
     1886have boolean values (rather than ``None``) if the field is a relation type
    18871887(:attr:`Field.is_relation=True <Field.is_relation>`).
    18881888
    18891889.. attribute:: Field.many_to_many

comment:13 by honya121, 9 years ago

Resolution: invalid
Status: assignedclosed

comment:14 by Tim Graham <timograham@…>, 9 years ago

Resolution: invalidfixed

In 2b08622:

Fixed #24665 -- Clarified model field flag defaults.

comment:15 by Tim Graham <timograham@…>, 9 years ago

In afae8ff:

[1.8.x] Fixed #24665 -- Clarified model field flag defaults.

Backport of 2b086229a2a6c786e32da37b6b122c2cc894450f from master

Note: See TracTickets for help on using tickets.
Back to Top