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 type hints to ann, meta #883

Merged
merged 2 commits into from
Mar 28, 2017
Merged

Conversation

janga1997
Copy link
Member

@janga1997 janga1997 commented Mar 9, 2017

#808
In ann.py, I am not really sure what the type of weights is supposed to be. I think it should be a list, but it could also be an numpy array? Any other iterable?
In meta.py , What is team's type supposed to be?
I am not sure, but I think these are the last of the strategies to add type hints to.

@marcharper
Copy link
Member

weights is a list of floats. A numpy array might work but I don't think we ever use it as such.

team is a list of player classes I think.

@drvinceknight
Copy link
Member

drvinceknight commented Mar 10, 2017

I'm not too sure why travis has not run on this: it's only showing 1 successful check (on Appveyor)...

@drvinceknight
Copy link
Member

I've run mypy locally and get:

Axelrod(janga1997-typeHint11): python run_mypy.py 
axelrod/strategies/ann.py:191: error: Argument 4 to "activate" has incompatible type List[int]; expected List[float]
axelrod/strategies/ann.py:195: error: Unsupported operand types for < ("int" and List[float])
axelrod/strategies/meta.py:94: error: Signature of "meta_strategy" incompatible with supertype "MetaPlayer"
axelrod/strategies/meta.py:109: error: Signature of "meta_strategy" incompatible with supertype "MetaPlayer"
axelrod/strategies/meta.py:211: error: Signature of "meta_strategy" incompatible with supertype "MetaPlayer"
axelrod/strategies/meta.py:249: error: Signature of "meta_strategy" incompatible with supertype "MetaPlayer"
axelrod/strategies/meta.py:390: error: Invalid type "axelrod.strategies.meta.NiceMetaWinnerEnsemble"
axelrod/strategies/meta.py:390: error: Invalid base class
axelrod/strategies/meta.py:402: error: Invalid type "axelrod.strategies.meta.NiceMetaWinnerEnsemble"
axelrod/strategies/meta.py:402: error: Invalid base class
axelrod/strategies/meta.py:413: error: Invalid type "axelrod.strategies.meta.NiceMetaWinnerEnsemble"
axelrod/strategies/meta.py:413: error: Invalid base class
axelrod/strategies/meta.py:424: error: Invalid type "axelrod.strategies.meta.NiceMetaWinnerEnsemble"
axelrod/strategies/meta.py:424: error: Invalid base class
axelrod/strategies/meta.py:435: error: Invalid type "axelrod.strategies.meta.NiceMetaWinnerEnsemble"
axelrod/strategies/meta.py:435: error: Invalid base class

This could be due to an updated version of mypy so try pip install -U mypy.

@janga1997
Copy link
Member Author

@drvinceknight The Signature errors have something to do with the @staticmethod decorator, although I don't know what.
And NiceTransformer imported from strategy_transformer.py,and used to define NiceMetaWinnerEnsemble is raising the errors.

@drvinceknight
Copy link
Member

@drvinceknight The Signature errors have something to do with the @staticmethod decorator, although I don't know what.
And NiceTransformer imported from strategy_transformer.py,and used to define NiceMetaWinnerEnsemble is raising the errors.

I've taken a brief look @janga1997 but I'm afraid I don't see what the right way to do it is here... I can try and take another look during the week at some point if you don't find a way to do it.

@marcharper
Copy link
Member

The staticmethod related error is literally saying that type signature of the child class meta_strategy method is different than the parent class, which is technically correct. It's just not clear why that is an error.

The second issue is a bit more subtle. The decorated classes are created at runtime so the type checker thinks that the base strategy type either doesn't exist or is invalid. This may be a limitation of mypy, and there may be a workaround where we declare these players to be instances or child classes of MetaPlayer.

@marcharper
Copy link
Member

@janga1997 Do you want break out the ann and apavlov changes so we can merge those while we sort the metaplayer issues?

@janga1997
Copy link
Member Author

@marcharper I've not been able to find the time recently.
I removed the commit for the meta.py file.

@drvinceknight
Copy link
Member

We've had some failures back:

59.35s$ python run_mypy.py
axelrod/strategies/meta.py:389: error: Invalid type "axelrod.strategies.meta.NiceMetaWinnerEnsemble"
axelrod/strategies/meta.py:389: error: Invalid base class
axelrod/strategies/meta.py:401: error: Invalid type "axelrod.strategies.meta.NiceMetaWinnerEnsemble"
axelrod/strategies/meta.py:401: error: Invalid base class
axelrod/strategies/meta.py:412: error: Invalid type "axelrod.strategies.meta.NiceMetaWinnerEnsemble"
axelrod/strategies/meta.py:412: error: Invalid base class
axelrod/strategies/meta.py:423: error: Invalid type "axelrod.strategies.meta.NiceMetaWinnerEnsemble"
axelrod/strategies/meta.py:423: error: Invalid base class
axelrod/strategies/meta.py:434: error: Invalid type "axelrod.strategies.meta.NiceMetaWinnerEnsemble"
axelrod/strategies/meta.py:434: error: Invalid base class

Add remaining type hints

type hint for distribution

Edits to ann.py

remove meta
@janga1997
Copy link
Member Author

@drvinceknight I forgot to remove meta.py from run_mypy

@drvinceknight
Copy link
Member

@drvinceknight I forgot to remove meta.py from run_mypy

Great! Travis is happy (and both ann.py and apavlov.py are being run through mypy: https://github.com/Axelrod-Python/Axelrod/blob/master/run_mypy.py#L14)

@meatballs meatballs merged commit 9878d0c into Axelrod-Python:master Mar 28, 2017
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.

4 participants