Given the additional types that can now be passed as penalty param I think it needs to change from

penalty: Optional[float] = None

to

penalty: Optional[Union[float, List[float]]] = None

where int is implied by virtue of having float there so does not need to be specific.

This type of checking here could check instead for list or numpy array and if not then put the quantity in a list. This would then avoid explicit float/int tests. We say a float, or List[float] upfront so it should be clear this is what should be passed - typehints state that int is implied too when you say float.. This way there is no a need to check on the contents. It may fail later when its used of course when the penalty is part of the expression. It seems a lot less explicit checking.

Thanks for the feedback. It seems like there is a trade off between how strict the test is for the outer iterable object and the test for inner elements.

I wanted to be as general as possible for the iterable objects so that other iterable types such as pandas.DataFrame or tuple can also be used. So I have to check the only types that we allow as a single value (i.e. int/float) before construct a list out of that iterable objects.

If on the other hand, we only want to support list and numpy array and want to be as general as possible for the inner element (e.g. Decimal, complex (not sure if that makes sense for penalty)) I think it would be fine to avoid explicit float/int test.

Also, is performance an issue we need to consider? It seems to me that type checking at the beginning is slightly more efficient then doing calculation in the python for loop and waiting for error.

In general you won't find this kind of type checking - especially over list contents. A user would be expected to provide input that conformed to being used as the stated kind, as per typehint/docstring etc. Evidently if it cannot be used in the expression as needed then Python will raise an exception.

If the user has an iterable object its really easy for them to make a list (as you do) before calling the method so given a input that takes a list, or a single instance of what is in the list, is not so onerous. Hence checking for list, and wrapping it if not as a list, is done elsewhere in this repo for example. Having an input of a list/ndarray is typically what is supported throughout - where this meets an end user, in my mind there is no real need to make the code here more complex I think to save the end user a line or two of very simple code to make a list if they have something else.

Performance in this case should not be an issue - I doubt these lists are very big. Either way in the normal case, that everything is correct, I would argue the code as you propose will be slower since it is always checking types before letting it be used.

What if only checked for iterable and wrapped if it is not one?

if not hasattr(penalty, "__len__"):
    penalty = [penalty] * num_constraint
elif len(penalty) == 1:
    penalty = [penalty[0]] * num_constraint

This also works with a list of single item.

I am not sure that is a good check for iterable. I thought about typehint of Iterable[float], but I believe there are aspects of typing around this are being deprecated in Python 3.9. Given a user has an iterable x they can simply do list(x) themselves to pass it in, and that seems like not too burdensome on an end-user and keeps the code here simple.

This is very reasonable, but I am also very much like passing iterables interchangeably to functions, as is with most Python libraries, specially lists and numpy arrays. Anyhow, for this code we need to check if the input is a single float/int (or a list of single item) then replicate that for all constraints. This convoluted the checking step.

Although I am not familiar with Qiskit codebase and convention, if I may offer my 2 cents, I would vote for general iterable since that seems to be the case for numpy and pandas. As for not checking for int/float, I think it is okay as long as there won't be problem caused later on (e.g. complex type) that might not throw an exception during calculation.

Please let me know if you guys want me to do just list/np.array or general iterable and whether to check int/float.

I think doing just list/np.array for checking is ok. Right now, I don't have any other use cases using general iterable.