Annotate normal_form_game.py #576

rht · 2021-04-16T03:18:28Z

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

QuantEcon.py/quantecon/game_theory/normal_form_game.py

Line 931 in 6ffbe57

return a

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

pep8speaks · 2021-04-16T03:18:30Z

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 · 2021-04-16T03:19:02Z

Will fix the line too long issues later.

coveralls · 2021-04-16T03:25:02Z

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

rht · 2021-04-16T03:41:31Z

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 · 2021-04-16T03:47:03Z

Just ignore the error?

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

oyamad · 2021-04-16T03:53:07Z

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:

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

Maybe option 1?

rht · 2021-04-16T03:53:38Z

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 · 2021-04-16T04:11:32Z

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?

rht · 2021-04-16T05:10:25Z

quantecon/game_theory/normal_form_game.py

@@ -233,7 +240,7 @@ def delete_action(self, action, player_idx=0):
        payoff_array_new = np.delete(self.payoff_array, action, player_idx)
        return Player(payoff_array_new)

-    def payoff_vector(self, opponents_actions):
+    def payoff_vector(self, opponents_actions: npt.ArrayLike) -> np.ndarray:


opponents_actions should have been IntOrArray, which is Union[int, npt.ArrayLike].
But doing so will cause problem in

QuantEcon.py/quantecon/game_theory/normal_form_game.py

Line 270 in d975adb

reduce_last_player(payoff_vector, opponents_actions[i])

. A Union[int, ...] type generally can't be indexed. Alternative solution to my current version is to use cast() before the indexing to cast opponents_actions into an indexable.

Note that int is also part of npt.ArrayLike, since np.array(1) works.

mmcky · 2021-04-30T10:49:22Z

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 · 2021-04-30T14:11:01Z

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 · 2021-04-30T14:30:46Z

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 · 2021-05-01T00:27:46Z

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 (

QuantEcon.py/quantecon/game_theory/__init__.py

Line 7 in 760dc3c

from .normal_form_game import pure2mixed, best_response_2p

) and is not used internally anywhere in the module.

mmcky · 2022-04-05T02:52:14Z

@oyamad are you happy with these annotations?

rht · 2022-04-05T07:58:38Z

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

QuantEcon.py/quantecon/game_theory/normal_form_game.py

Lines 258 to 261 in 538bf7c

    
           if isinstance(action, numbers.Integral):  # pure action 
        
               return payoff_array.take(action, axis=-1) 
        
           else:  # mixed action 
        
               return payoff_array.dot(action)

.

Copilot

Pull Request Overview

This PR adds type annotations to the normal_form_game.py module to improve type safety and developer experience. The changes include comprehensive type hints for function parameters, return types, and class attributes while also fixing a potential bug where a function could return None instead of the expected int.

Adds comprehensive type annotations throughout the Player and NormalFormGame classes
Introduces a custom type alias IntOrArrayT for union types commonly used in the module
Fixes a missing return statement bug in the best_response_2p function

_{You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.}

Copilot · 2025-08-16T01:06:38Z

quantecon/game_theory/normal_form_game.py

-def best_response_2p(payoff_matrix, opponent_mixed_action, tol=1e-8):
+def best_response_2p(
+    payoff_matrix: np.ndarray, opponent_mixed_action: np.ndarray, tol: float = 1e-8
+) -> int:


The function best_response_2p has a missing return statement. Based on the PR description, there's a case where the function can return None instead of int when the if condition is never satisfied. The function needs a proper return statement to handle all code paths.

The annotation here is quite old (from typing import List instead of using list). Can you also highlight outdated patterns in this PR?

@copilot see my previous message.

@copilot can you please review @rht comments above.

coveralls · 2025-08-16T23:15:34Z

coverage: 92.629% (+0.003%) from 92.626%
when pulling f731bc8 on rht:master
into 6045b51 on QuantEcon:main.

Annotate normal_form_game.py

b2a8f98

rht commented Apr 16, 2021

View reviewed changes

rht mentioned this pull request Apr 16, 2021

Use Mypy annotation for arguments and return type documentation #565

Open

oyamad self-assigned this Apr 5, 2022

mmcky requested a review from Copilot August 16, 2025 01:06

Copilot AI reviewed Aug 16, 2025

View reviewed changes

mmcky assigned Copilot Aug 16, 2025

Merge branch 'main' into master

f731bc8

Uh oh!

Annotate normal_form_game.py #576

Are you sure you want to change the base?

Annotate normal_form_game.py #576

Uh oh!

Conversation

rht commented Apr 16, 2021

Uh oh!

pep8speaks commented Apr 16, 2021

Uh oh!

rht commented Apr 16, 2021

Uh oh!

coveralls commented Apr 16, 2021

Uh oh!

rht commented Apr 16, 2021

Uh oh!

mmcky commented Apr 16, 2021

Uh oh!

oyamad commented Apr 16, 2021

Uh oh!

rht commented Apr 16, 2021

Uh oh!

rht commented Apr 16, 2021

Uh oh!

rht Apr 16, 2021

Choose a reason for hiding this comment

Uh oh!

mmcky commented Apr 30, 2021

Uh oh!

oyamad commented Apr 30, 2021

Uh oh!

oyamad commented Apr 30, 2021

Uh oh!

rht commented May 1, 2021

Uh oh!

mmcky commented Apr 5, 2022

Uh oh!

rht commented Apr 5, 2022

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Aug 16, 2025

Choose a reason for hiding this comment

Uh oh!

rht Aug 16, 2025

Choose a reason for hiding this comment

Uh oh!

rht Aug 16, 2025

Choose a reason for hiding this comment

Uh oh!

mmcky Aug 16, 2025

Choose a reason for hiding this comment

Uh oh!

coveralls commented Aug 16, 2025

Uh oh!

Uh oh!