Allow vars to be set to null and differentiate them from unset vars (#608) #1426
Conversation
There was a problem hiding this comment.
I think the original var() context function was added here. It looks to me like this was an oversight -- because the default value for default was set to None, we couldn't distinguish between a supplied var with a value of None and the absence of a supplied var.
The test you referenced was added here and I think this just matched the existing behavior, but I don't see any big insight behind why this needs to be the case.
All told, I struggle to think of a reason why we wouldn't want to support None as a valid variable value, and this all L very GTM
One question below though for my own edification
| ) | ||
|
|
||
| def __call__(self, var_name, default=None): | ||
| def __call__(self, var_name, default=_VAR_NOTSET): |
There was a problem hiding this comment.
As I understand it, supplying an object (or non-primitive) as a default argument means that Python uses the pointer to that object as the default value for all invocations of a method. This is the thing you're taking advantage of here, right?
This might be way off the mark, but... is this... thread-safe? Is _VAR_NOTSET initialized when the common.py file is imported? This is a neat construct!
There was a problem hiding this comment.
Mostly! Python actually guarantees that supplying any value as a default argument means that Python uses that object as the default value. The tricky part that confuses people is that there is in fact only one each of None, True, and False. The id() of an object is guaranteed to be unique, and a is b basically checks id(a) == id(b).
This is a somewhat common technique for handling this irritating case of wanting to distinguish between None and "not set" on an optional arg. The other way I've seen is doing **kwargs and checking what's in there, which I'm not really fond of.
I'm not sure where I first stumbled across it, but I have been using it for a long time and I sure didn't come up with it on my own. Because it's the default argument, the user can never intentionally pass it, so it can only arrive on a function signature. I don't think it's possible to go wrong here from a thread/process safety perspective. You probably shouldn't subclass and override this method or the value though!
If you need even more comfort: Because the variable is defined at the class level, and the class is defined at the module level, the identity of the object is defined at module import time. Python guarantees that module loading is thread-safe by having a lock around imports, so you know that this value will only be set once and be visible to all threads.
|
Ha, I added it! Well that's a nice explanation for why there's a test of something we don't care about! |
Fixes #608
Fix #608 by distinguishing between
Nonevariables and not having a variable set at all.Potentially of interest - I had to remove a test that explicitly ensured that we did not allow variables to be
None. I assume that All Tests Exist For A Reason, though of course the reason might very well be "we used to think this was a good idea and now we don't".