Minor cleanup

[dandavison]

Some minor cleanup pulled out of Nexus work.

[cretz]

Interesting, can the static analyzer know that this is set before used? Or do the Python static analyzers not check whether a value is set before use?

[dandavison]

They can't detect all situations where it is read before being set. So in that case it would be a run-time error, similar to the AssertionError that would have been thrown by the code prior to this commit. Since the code algorithmically guarantees that these run-time errors do not in fact occur, IMO it is more idiomatic and cleaner not to set an initial value that is never used.

[cretz]

IMO it is more idiomatic and cleaner not to set an initial value that is never used

Never used now but you have no protection anymore. I think it's safer to only ever capture variables in a nested function that have been initialized, even if you promise to re-initialize them again later. And nonlocal and assert signal this intent.

[dandavison]

I think it's safer

How is it safer -- it's a run-time error either way, either an AssertionError or a NameError / some sort of unbound variable error. Either way, the problem will be simple to track down.

[cretz]

It's safer from a general logic POV to have no possibility of uninitialized variable access (in any programming language). And it's also clearer from a reader POV that you intended for this situation via nonlocal/assert, which helps safety/comprehension. Python is not smart enough to enforce accessing an only initialized variables like some statically typed languages do, hence the help. Sure it may not matter much here, definitely not enough to change from what it was, but I'm not pushing back on you changing it.