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

Add Original Gradual to the list of all strategies. #1332

Merged
merged 9 commits into from Apr 15, 2020

Conversation

drvinceknight
Copy link
Member

@Nikoleta-v3 ran the full tournament and realised that I'd missed adding Original Gradual to the list of strategies.

@marcharper
Copy link
Member

I'm surprised no doc test failed -- could we think of one, maybe just a count of all the strategies?

@drvinceknight
Copy link
Member Author

I'm surprised no doc test failed -- could we think of one, maybe just a count of all the strategies?

I like the idea of doctesting the number of strategies in the library (I think we've discussed this before). We could have something here https://github.com/Axelrod-Python/Axelrod/blob/master/docs/index.rst just under the "over 230" strategies, we might as well say exactly how many there are?

FWIW, this isn't the first time this has happened (I usually double check the new strategies are in the results from the last run of the tournament).

@drvinceknight
Copy link
Member Author

drvinceknight commented Apr 15, 2020

@marcharper what do you think of 70bffa6 (adding a simple count of all strategies to docs)?

@gaffney2010
Copy link
Member

I'm surprised no doc test failed -- could we think of one, maybe just a count of all the strategies?

In this case, OriginalGradual was imported and never used. Maybe we could check for that. May be overkill, but could check other errors.

pylint --disable=all --enable=unused-import axelrod/strategies/_strategies.py

[Actually when we start adding logic for classifiers, a test of classifier equality on a strategy would break if the strategy isn't in all_strategies, because it never gets recorded in the yaml.]

@drvinceknight
Copy link
Member Author

In this case, OriginalGradual was imported and never used. Maybe we could check for that. May be overkill, but could check other errors.

pylint --disable=all --enable=unused-import axelrod/strategies/_strategies.py

Not a bad idea at all, there's no guarantee that there's not another strategy that we've missed in the past. I'll give this a go.

@drvinceknight drvinceknight force-pushed the add-original-gradual-to-list branch 2 times, most recently from 9f36e65 to 924c793 Compare April 15, 2020 11:02
@drvinceknight
Copy link
Member Author

@gaffney2010 I've added that check to the CI (I believe the commits above are clear but let me know if I can explain). Note that in some cases there are correct occurrences of classes being imported but not used. Have had to annotate those (and separate their imports), I've documented that in the module doctoring. Let me know what you think 👍

@Nikoleta-v3 Nikoleta-v3 merged commit d608fdd into master Apr 15, 2020
@marcharper marcharper deleted the add-original-gradual-to-list branch April 16, 2020 22:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants