351 bug check for chance information sets in mixed behavior profiles #377
Conversation
src/pygambit/tests/test_behav.py
Outdated
|
|
||
| def test_infoset_value_with_chance_player_infoset(self): | ||
| """Test to ensure that infoset_value called with an infoset of the chance player | ||
| raises a ValueError""" |
There was a problem hiding this comment.
Style for triple-quoted strings that are over more than one line usually put the closing triple quote on its own line.
src/pygambit/behav.pxi
Outdated
| return self._payoff(self.game._resolve_player(player, 'payoff')) | ||
| resolved_player = self.game._resolve_player(player, 'payoff') | ||
| if resolved_player.is_chance: | ||
| raise ValueError("It is not possible to call payoff for the chance player") |
There was a problem hiding this comment.
I would suggest saying "payoff() is not defined for the chance player". After all it's possible to call the function (the calling code just did so!). It's just that it's not a well-defined concept. Similar comment on the other functions.
src/pygambit/tests/test_behav.py
Outdated
| @@ -1,5 +1,4 @@ | |||
| import unittest | |||
|
|
|||
There was a problem hiding this comment.
The space between the imports was intentional. The general pattern is
imports of standard library
<blank line>
imports of third-party modules
<blank line>
imports of modules from this package
There was a problem hiding this comment.
Understood, thanks.
Actually, I did not remove a blank link. I included the unittest import, along with inheriting from unittest.TestCase, which was not done only for test_behav.py and test_exten.py when I created my branch. I see that you also fixed this for test_behav.py and moved test_exten.py to pytest.
There was a problem hiding this comment.
I see in the changelog: "Migrated to pytest for testing of pygambit."
There's a few that have not been converted: test_behav.py, test_nash.py, test_game_resolve_functions.py, test_file.py.
…w value errors when called with the chance player
rahulsavani
force-pushed
the
351-bug-check-for-chance-information-sets-in-mixed-behavior-profiles
branch
from
5457a20 to
fc9734a
Compare
tturocy
deleted the
351-bug-check-for-chance-information-sets-in-mixed-behavior-profiles
branch
I have implemented the discussed changes and believe the code in the final commit is good to be merged now; happy to rebase if you suggest that, and to a changelog entry, but would need some guidance on the latter..