In terms of raising the
KeyError as soon
cluster_terminals is called (rather than when its result is iterated), this general pattern is likely the best. There are a couple of other points that bear mentioning, though.
assert False is one of the worst ways to handle a detected error (marginally better than, say,
sys.exit(0)). Make the assertion self-documenting so that
if when it gets triggered sometime in the future, you have more immediate information in the stack trace about what is going on. At a minimum, include a message with the assertion:
assert False, "`return_list` contains unexpected object of type " + str(type(x))
An arguably better option is to put it up front with an explicit test, even if that means duplicating some of your logic:
assert all(isinstance(x, (ClusterTerminal, NodeTerminalMappingName)) for x in return_list)
And then adjust your conditional to either have a
pass (and a short comment pointing to the assertion) in that branch, or omit that branch entirely. This makes it abundantly explicit to anyone reading your code that that is an 'impossible' situation, and means the invariants of this class have been broken (and that it isn't that client code has passed in bad data - for that, use a normal exception rather than an assertion).
A more minor quibble is that you don't need to pass
return_list into the helper function as an arguments. You can use closures:
return_list = self.cluster_map[key]
This will behave identically - Python functions can see variables from the scope they are defined in, even if that scope no longer 'exists' when they are executed.
internal_cluster_terminals is a bit of an unwieldy name, and doesn't tell us anything we don't already know (it's.. internal to the function it's defined in. Wow, amazing). A more usual name for this type of function is something like
return_list isn't a great name either, especially since it's actually misleading (it isn't ever
returned). Since it's a value from a mapping, call it after what those values conceptually are -
cluster might be a good option.
Finally, consider adding a short comment to document why you need a helper function in the first place, like:
# Use an internal generator instead of yielding directly
# so we can bail out early if there's a KeyError