Skip to content
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

Modify/fix initial play of lookerup players. #1190

Merged
merged 1 commit into from
Aug 15, 2018

Conversation

drvinceknight
Copy link
Member

Closes #1185

Currently, the lookerup players pop from a list of initial plays. This
breaks when playing against the cheating players which simulate their
play (as the list of initial players no longer exists).

This fixes that by ensuring the list of initial plays does not
disappear which I think technically is probably better behaviour anyway?

Closes #1185

Currently, the lookerup players pop from a list of initial plays. This
breaks when playing against the cheating players which simulate their
play (as the list of initial players no longer exists).

This fixes that by ensuring the list of initial plays does not
disappear which I think technically is probably better behaviour anyway?
@marcharper marcharper added the bug label Aug 14, 2018
@marcharper
Copy link
Member

Looks fine assuming the Appveyor build finishes successfully.

@drvinceknight
Copy link
Member Author

drvinceknight commented Aug 14, 2018

Looks fine assuming the Appveyor build finishes successfully.

The failure is related to #1187, I'm actually beginning to suspect that has to do with the Human player and prompt toolkit. I'm seeing if I can figure that out, but a simple fix might just be to remove the Human player for now.

EDIT: Almost certain it is not the Human player.

@drvinceknight
Copy link
Member Author

As far as I can tell it's not possible to rerun branch builds on appveyor (as opposed to PR builds). So I suggest this is merged @marcharper, I've opened #1191 to try and identify what is causing the issue.

@drvinceknight
Copy link
Member Author

I'm labelling this as ready to merge as I do not see how to restart the branch build on appveyor (I'm continuing to see if I can figure out #1187).

@marcharper
Copy link
Member

You can start a new build with the "Play" icon button on Appveyor.

I think that the error may be due to matplotlib and our after tournament plots. I've read elsewhere that using matplotlib.use('agg') may help. The default backend is tkAgg (tkinter), which uses Tcl.

Alternatively we could suppress "showing" the images in the test suite, which I think could fix the issue.

@meatballs meatballs merged commit 9447a40 into master Aug 15, 2018
@meatballs meatballs deleted the fix-cheating-strategies-bug branch August 15, 2018 06:36
@drvinceknight
Copy link
Member Author

You can start a new build with the "Play" icon button on Appveyor.

I'm used to that Play button and have used it many times on PR builds but as far as I can see it doesn't exist on specific commit builds, I spent a long time double checking that but if/when this happens again, you could screenshot where it is on a commit build that would be awesome. 👍

@marcharper
Copy link
Member

I just clicked on the Details link for "continuous-integration/appveyor/branch AppVeyor build failed" above and the page that it takes me to first has the Play "New Build" button showing. Maybe you were signed out or signed into a different account and couldn't see it for some reason (I had to sign in with github to see it).

@drvinceknight
Copy link
Member Author

Yeah it seems to be there sometimes and sometimes not (???).

The build link from the failed thing:

https://ci.appveyor.com/project/marcharper/axelrod/build/1.0.330

Gives me:
screenshot from 2018-08-16 07-57-08

Not important.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

all_strategies tournament fails
3 participants