-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Annotate normal_form_game.py #576
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Hello @rht! Thanks for opening this PR. We checked the lines you've touched for PEP 8 issues, and found:
|
|
Will fix the line too long issues later. |
There is never such case, from the way |
In |
Yes, it happens when There are two options:
Maybe option 1? |
|
There is a |
Maybe it is safer to tell the user there is a problem instead of |
| return Player(payoff_array_new) | ||
|
|
||
| def payoff_vector(self, opponents_actions): | ||
| def payoff_vector(self, opponents_actions: npt.ArrayLike) -> np.ndarray: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
opponents_actions should have been IntOrArray, which is Union[int, npt.ArrayLike].
But doing so will cause problem in
| reduce_last_player(payoff_vector, opponents_actions[i]) |
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.
thanks @rht sorry for my slow response.
@oyamad should we add an exception if |
Actually, this function |
So your annotations are correct. |
|
|
@oyamad are you happy with these annotations? |
|
I think it is safe to replace all of QuantEcon.py/quantecon/game_theory/normal_form_game.py Lines 258 to 261 in 538bf7c
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request adds comprehensive type annotations to the normal_form_game.py module in the QuantEcon library. The PR addresses type safety by adding explicit type hints throughout the module and fixes a missing return statement bug.
Key changes:
- Added comprehensive type annotations to all classes, methods, and functions
- Introduced a custom type alias
IntOrArrayTfor flexibility with int/array parameters - Fixed a missing return statement bug in the
best_response_2pfunction
| 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: |
Copilot
AI
Aug 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function best_response_2p is missing a return statement implementation. The function signature indicates it should return an int, but there's no function body shown that would ensure this return type is always satisfied.
| """ | ||
| def __init__(self, data, dtype=None): | ||
| def __init__(self, data: npt.ArrayLike, dtype: Optional[np.dtype] = None): |
Copilot
AI
Aug 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The parameter data is annotated as npt.ArrayLike but the implementation shows it can also accept an array of Player objects. The type annotation should be Union[npt.ArrayLike, Sequence[Player]] to accurately reflect all accepted input types.
| def __init__(self, data: npt.ArrayLike, dtype: Optional[np.dtype] = None): | |
| def __init__(self, data: Union[npt.ArrayLike, Sequence[Player]], dtype: Optional[np.dtype] = None): |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
Latest changes:
|
np.ndarrayis a catch-all type for all ndarrays regardless of its dtype, and so, is unfortunately less specific than the docstring that specifies ndarray.action_profilesince the functions using it are not documented.normal_form_game.py:921: error: Missing return statementinQuantEcon.py/quantecon/game_theory/normal_form_game.py
Line 931 in 6ffbe57
Noneinstead ofintwhen the if condition is never satisfied.