Proposal: The Undefined singleton should by falsey

[tmreay]

I noticed some unintuitive behavior while working on a HasTraits object with a ReadOnly attribute.

I expected the default value, the Undefined singleton, to be falsey, but it is not. See below:

In [1]: from traits.api import ReadOnly, HasTraits

In [2]: class A(HasTraits):
   ...:     val = ReadOnly
   ...:

In [3]: a = A()

In [4]: if a.val:
   ...:     print('{} evaluates to True, so lets operate on it'.format(a.val))
   ...: else:
   ...:     print('{} evaluates to False, so lets assign it'.format(a.val))
   ...:
<undefined> evaluates to True, so lets operate on it

To properly check if the value is set, I need to import Undefined and then do if a.val is Undefined, which isn't terrible, but I'd like the option to just treat it as a bool.

I think all that's needed is to define a __nonzero__ method (and __bool__ for python 3) that returns False

Thoughts?

[corranwebster]

Not necessarily opposed to this, but not sure what impact it may have.

Is there a reason why an explicit if a is Undefined won't work? This may be preferable for the same reason if a is None is preferred.

[itziakos]

I think that the fact the Undefined tranlates to true it the bug. When a Trait has the Undefined value this value should fail when used or checked for true or false. Using a trait with an Undefined value should be an error and only checking for Undefined makes sense.

Please also note that the ReadOnly trait is a kind of special trait which does not have a type and there is no default value for it like we have for traits like List, Bool and Int

[tmreay]

To @itziakos's point, I agree that Undefined translating to true is the underlying bug. I view it as conceptually similar to None, which is why I'd prefer it to evaluate to false, but I would be fine with instead raising an error if the user tries to coerce it's value to a bool.

@corranwebster Theres no reason the is check would not work, I was just wanting to avoid an extra import. In my use case, the value that would be assigned to this trait would always be truthy, so there would not be any ambiguity to treating it like a bool.

[itziakos]

Just to elaborate on my thoughts about Undefined. This specific value designates that there has been no value defined for that Trait at initialization. I find it different than None which is a valid default value for a number of traits (e.g. Instance). So traits/hastraits internals can destinguish between non-assigned traits without any ambiguity and call the default trait handler (if available).

[rkern]

None being falsey has been a consistent trap for people, causing them to write if x: when if x is not None: is actually required. You really should not test the truthiness of your object to determine whether or not it is Undefined (or None). Even if you do expect your real values to alway be truthy, requirements evolve, and in the future your real values may be falsey. By using if a.val:, you have imposed a constraint on the future evolution of the code, but in a silent way that only shows up in an obscure bug later.

I'm not saying that Undefined should not be falsey (the analogy to None is apt), but I do think that you should never rely on it being falsey or truthy. You should always test if a value is Undefined by an is Undefined test. I don't think there is an actual use case to motivate this, just the theoretical analogy to None.

[tmreay]

@itziakos @rkern All valid points. If I'm going to draw the mental analogy between Undefined and None, that should extend to always checking for Undefined with is Undefined.

The more I've thought about this, I think I'm actually in favor of the solution @itziakos proposed, raising an error if coerced to a bool.

In an effort to avoid this discussion turning into a case of bike-shedding, I think it's probably best to rename this issue "The Undefined singleton is truthy", replace the "enhancement" tag with a "bug" tag, and maybe add a "low priority" tag (I don't see any other "low priority" tags in this repo). Or drop it entirely

[mdickinson]

replace the "enhancement" tag with a "bug" tag

That wouldn't make sense here: this isn't a bug, unless there's something in the documentation or elsewhere that indicates that Undefined was intended to be falsy.

I'd favour leaving this alone.

[tmreay]

@mdickinson Fine by me, closing now.