Annotate normal_form_game.py

[rht]

• np.ndarray is a catch-all type for all ndarrays regardless of its dtype, and so, is unfortunately less specific than the docstring that specifies ndarray.
• I'm unsure of the type of action_profile since the functions using it are not documented.
• I found this bug: normal_form_game.py:921: error: Missing return statement in https://github.com/QuantEcon/QuantEcon.py/blob/6ffbe570008979e22b3156dd77745990b53ac74d/quantecon/game_theory/normal_form_game.py#L931. There is a case where the function returns None instead of int when the if condition is never satisfied.

[pep8speaks]

Hello @rht! Thanks for opening this PR. We checked the lines you've touched for PEP 8 issues, and found:

• In the file quantecon/game_theory/normal_form_game.py:

Line 204:80: E501 line too long (82 > 79 characters) Line 259:80: E501 line too long (92 > 79 characters) Line 525:9: E125 continuation line with same indent as next logical line Line 525:80: E501 line too long (86 > 79 characters) Line 732:80: E501 line too long (94 > 79 characters) Line 759:80: E501 line too long (86 > 79 characters) Line 817:80: E501 line too long (90 > 79 characters) Line 922:80: E501 line too long (83 > 79 characters)

[rht]

Will fix the line too long issues later.

[coveralls]

Coverage increased (+0.003%) to 94.338% when pulling b2a8f9856e1a15163a2d31ed949633880a19fdb2 on rht:master into 6ffbe570008979e22b3156dd77745990b53ac74d on QuantEcon:master.

[rht]

There is a case where the function returns None instead of int when the if condition is never satisfied.

There is never such case, from the way payoff_max is specified, but Mypy isn't aware of this. Just ignore the error?

[mmcky]

Just ignore the error?

In Mypy is there a way to add an ignore such as # noqa for coveralls?

[oyamad]

There is a case where the function returns None instead of int when the if condition is never satisfied.

Yes, it happens when tol < 0. (tol is supposed to be nonnegative, but it is just implicit.)

There are two options:

1. Just return -1 when the if condition is never satisfied (which happens if and only if tol < 0).
2. Let tol = 0 if tol < 0.

Maybe option 1?

[rht]

There is a type: ignore for a single line, but the error appears exactly at the def best_response_2p(...) line. If this line is disabled, the entire function signature's type checking will also be disabled.

[rht]

Just return -1 when the if condition is never satisfied (which happens if and only if tol < 0).

Maybe it is safer to tell the user there is a problem instead of -1? I tried raising an Exception at the end and Mypy no longer complains about missing a return. What should the exception message be?

[mmcky]

Just return -1 when the if condition is never satisfied (which happens if and only if tol < 0).

Maybe it is safer to tell the user there is a problem instead of -1? I tried raising an Exception at the end and Mypy no longer complains about missing a return. What should the exception message be?

thanks @rht sorry for my slow response.

tol is supposed to be nonnegative, but it is just implicit

@oyamad should we add an exception if tol is negative instead and then the return is bounded.

[oyamad]

should we add an exception if tol is negative instead

Actually, this function best_response_2p is for internal use. Negative tol should be handled by the outer function that calls best_response_2p. Let's just return some int, say -1, and write in the docstring, something like "Return -1 if there is no best response action, which occurs when tol < 0".

[oyamad]

I'm unsure of the type of action_profile since the functions using it are not documented.

• action_profile in __getitem__ and __setitem__ is int if N=1 and array_like if N>=2.
• action_profile in is_nash is array_like.

So your annotations are correct.

[rht]

should we add an exception if tol is negative instead

Actually, this function best_response_2p is for internal use. Negative tol should be handled by the outer function that calls best_response_2p. Let's just return some int, say -1, and write in the docstring, something like "Return -1 if there is no best response action, which occurs when tol < 0".

best_response_2p is currently exported as an external function (https://github.com/QuantEcon/QuantEcon.py/blob/760dc3c7ceec078dfded4292d9ef9c68e68e33ce/quantecon/game_theory/init.py#L7) and is not used internally anywhere in the module.

[mmcky]

@oyamad are you happy with these annotations?

[rht]

I think it is safe to replace all of IntOrArrayT with npt.ArrayLike? The code selection part that checks whether actions is a scalar or array happens in https://github.com/QuantEcon/QuantEcon.py/blob/538bf7c7fa4fdbd1da5e7630c12667f5e6288fef/quantecon/game_theory/normal_form_game.py#L258-L261.