Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added support for null=True (and default=None) to BitField class. #14

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

eswenson1
Copy link

Also added a test that checks creation and manipulation of model instances with a BitField that uses null=True and default=None.

Prior to this change, the following model definition would not work:

class Foo(models.Model):
     flags=BitField(flags=FLAGS, null=True, default=None)

the resulting instances would ignore the default None, and have fields with BitHandler instances with all bits disabled.

Now, BitField fields can either be None (null) or BitHandler instances.

to include a test for null=True and default=None, and to test for
various manipulations of model instances with None as the value
of the BitField.
@dcramer
Copy link
Collaborator

dcramer commented Mar 21, 2012

Should we really allow them to stay NULL? My concerns around that are you wont be able to access foo.key_name, you'd have to say if foo and foo.key_name in many areas

@eswenson1
Copy link
Author

Hi David,

I believe I accidentally sent a partially composed message -- sorry 'bout
that.

In our application, we need to be able to distinguish between a field whose
value has never been set, or which wants to represent an unspecified set of
bit values. Simply using a BitHandler with all bit values set to False
doesn't represent the same thing as a default or null bit vector. For
example, we are representing a set of permission (the BitField/BitHandler)
on various objects in a hierarchy, where an intermediate object can decline
to provide an overridden set of permissions, causing the permission
calculation to check for default permissions up the hierarchy. Null
represents the lack of specific permissions, whereas the BitHandler value
represents a set of permissions (enabled and/or disabled flags). The value
null (None) can be both an initial value, or a BitHandler that has had its
overriden permissions removed, reflecting the need to look up the hierarchy
for permissions set at a higher level.

So, at least in our application, we require a value to be able to take on
either None or a BitHandler. This seems consistent with other model types,
where null represents the absence of a value, whereas non-null represents a
real value of the specified type. Take a BooleanField with null=True,
default=None. This field can take on three values: None, True, False.
These can be stored persistently and are all meaningful values. The same
is true with the BitField, at least in my view and in my implementation.

To be fair, one could make the argument that the real parallel to the
tri-state BooleanField would be one where each of the bits in the BitField
could take on three values (including Null). But this would be more
complicated and wasn't something that we need in our application. If you
believe that my BooleanField argument doesn't really apply in the case of
the BitField, I could understand this point of view.

Now, to your point of notational convenience and awkwardness of code: I
agree with you. Since we wanted this quasi-tristate, this inconvenience
was acceptable. And we do indeed say if foo and foo.key_name in various
places. The only argument I can make here is: you don't have to ever use
null=True. If you don't, and you always set a default value or initialized
value for the BitField, you won't have this ugliness in code that uses
BitField. If you do want the tri-state behavior, you don't have much
choice.

We're happily using the modified implementation now. However, if you
really object to this, I understand and will add additional fields to our
models to represent the set/not-set nature of the BitField, and revert my
changes to django-bitfield.

Take care, and thanks for taking the time to look into this. -- Eric

On Tue, Mar 20, 2012 at 5:02 PM, David Cramer <
[email protected]

wrote:

Should we really allow them to stay NULL? My concerns around that are
you wont be able to access foo.key_name, you'd have to say if foo and
foo.key_name in many areas


Reply to this email directly or view it on GitHub:
#14 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants