MAINT: Removed supurious assert in histogram estimators

[madphysicist]

This assert is superfluous. Both possible usages are already being handled:

1. The function is nominally private for use by histogram. In this case it is always called in an if isinstance(bins, basestring): block (Line 346 in master)
2. In the unlikely event that someone calls this function externally (e.g. to get access to the estimator functionality), the except KeyError: block on Line 157 will handle the situation better than the assertion since it will not be optimized away and will actually deliver a useful message.

[charris]

Thanks @madphysicist . Should get rid of the try .. except also.

[madphysicist]

Which try...except?

[charris]

Same function around dictionary access. Could check for key and raise proper error.

[charris]

Strictly speaking, the whole function should be removed, the methods made private functions, and the dictionary instantiated on module load.

[madphysicist]

Isn't the try...except doing exactly that? Checking for key and raising the proper error? It looks fine to me. I agree that the dictionary can be instantiated on load.

[charris]

Using try ... except for ordinary flow control is ugly and the code is not self documenting like it would be with an explicit key check and the else is a waste of bytes. I expect the the try .. except also consumes excess cpu cycles. Just as assert is mistakenly used for flow control, I think the try .. except is often misused for flow control. It makes sense perhaps for duck typing or errors like missing files or calling a complicated function with error returns, but here there is no need here and checking the key is simpler.

[madphysicist]

Just so I understand, you mean doing if key in dict: do lookup; else: raise?

[charris]

Well, I'd just do if key not in dict: raise :) Same difference. But if we move the dictionary out of the helper function the whole function can go away, so nothing pressing.

[madphysicist]

I am not sure that using the exception is going to incurr any significant overhead. I have read in a number of places that Python exceptions are quite cheap that way, unlike say Java. However, I do have to agree that using if key not in dict: raise is much nicer to read. See here: #7199.