-
Notifications
You must be signed in to change notification settings - Fork 282
an idea for Geller, Darwin, MindReader #950
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
Conversation
drvinceknight
left a comment
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.
Seems like a good approach. 👍
I noticed one of your commit messages about mypy, any problems running that locally? (It's pip installable).
axelrod/_strategy_utils.py
Outdated
| update_history(player_2, h2) | ||
|
|
||
|
|
||
| def cheating_bastard(cheater, opponent): |
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.
This function name will need to change. Similarly cheater needs to be something more verbose, I've been using the word cheater as shorthand, but technically it's just that it does not obey Axelrod's original rules.
I suggest:
def simulated_strategy(player, opponent):
```
Returns the simulated strategy of a player against an opponent. If the strategy has an implemented `simulation_strategy` then that gets called.
```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.
Can you also write one or two tests for this please.
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.
no problem. i'll put some work in making this more presentable, make tests for the simulated_strategies, and some tests for cheating_bastard (are you sure that name needs changing? ;) ).
names:
inspect_strategy(inspector, opponent):foil_strategy_inspections(self, opponent):
how do those sound?
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.
I noticed one of your commit messages about mypy, any problems running that locally? (It's pip installable).
i had some weird issues that took working out. for some reason, the mypy command doesn't make doesn't add to any kind of PATH variable. so, with all this installed in a virtualenv i had to write a shell script pointing directly to a batch file the environment's Scripts folder. all in all, the workaround needed
- a python script to get names from run_mypy.py into a csv file
- the csv file
- a shell script to run the first one, load the second one and then run each variable through a path to the mypy.bat file.
not pretty, but i've barely worked with bash before, so that was useful.
i also had issues understanding what the problem was. i was not aware of the variable declaration as an inline comment.
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.
Eeeesh
names:
inspect_strategy(inspector, opponent):
foil_strategy_inspections(self, opponent):
how do those sound?
That sounds good to me 👍
i also had issues understanding what the problem was. i was not aware of the variable declaration as an inline comment.
Eeeesh that sounds difficult, I can't offer any assistance I'm afraid...
| will cooperate. | ||
| """ | ||
| name = 'Geller Cooperator' | ||
| default = lambda self: C |
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 IDE balked at making a variable a lambda and recommended changing to a function. so i did.
|
|
||
| self.versus_test(axelrod.MindReader(), expected_actions=[(C, D)] * 2, attrs={'genome': [D, C]}) | ||
|
|
||
| def test_play(self): |
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.
when i refactored test_darwin, i forgot to recommend removing test_play(self). Because versus_test calls "play", these are now redundant.
|
for the test_ modules, i did not refactor the modules. i only added the test for foil_strategy_inspection. i'll be refactoring test_geller in the next few days in a PR to #884. test_darwin just got refactored and test_mindreader will be handled by someone (possibly me). |
drvinceknight
left a comment
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.
Just 3 very minor 1 line doc string requests from me.
| if inspect.stack()[1][3] not in Darwin.valid_callers: | ||
| return C | ||
| @staticmethod | ||
| def foil_strategy_inspection() -> Action: |
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.
Could you add a docstring here please.
| } | ||
|
|
||
| @staticmethod | ||
| def foil_strategy_inspection() -> Action: |
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.
Docstring.
| reader or bender by cooperating. | ||
| """ | ||
| @staticmethod | ||
| def foil_strategy_inspection() -> Action: |
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.
Docstring please.
#941
okay. an idea flashed into my head on how to solve this without affecting anyone but the three cheaters. i spent all of 10 minutes implementing it (as you'll see by the naming and commented-out code.)
if the general idea is ok, i think it's worth spending the time to make this a presentable PR. aside from getting that same feeling as when i try to walk past an unfinished jigsaw puzzle, using inspect.stack() is just asking to be broken. if anything changes in play, it may or may not require some weird maintenance for these guys. this should solve that.
the changes only involve the strategy_utils function written specifically for the cheaters, so no one else should know or ever care about the code change there.
the other changes are adding a specific method to the cheaters to deal with cheaters that, once again, the whole rest of the code can remain blissfully ignorant of.