Skip to content

Add type hints for plot #833

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

Merged
merged 14 commits into from
Feb 3, 2017
Merged

Conversation

janga1997
Copy link
Member

@janga1997 janga1997 commented Jan 29, 2017

#808
@marcharper @drvinceknight Obviously there's a lot left. But I need to discuss before attempting any further.

  • The parameter data I think, can be list of ints, floats, etc. I think it would be better to just do data: list.

  • The parameter title too can be an int, float, and object, list or anything. I can't think of way to assign a broad enough type for it.

  • I think the type of the parameter ax is matplotlib.axes._subplots.AxesSubplot, although I am not sure.

  • I don't understand what is names.

@marcharper
Copy link
Member

I think you've got ax right; title should be a strings and names is probably a list of strings.

@janga1997
Copy link
Member Author

And data?

@marcharper
Copy link
Member

Looking through the code it looks like it's a list of lists of floats (look at self._boxplot_dataset which creates it in one case). However I didn't check every instance so it might be different for some of the other plots.

@janga1997
Copy link
Member Author

Yes, but it could very well be a list of list of ints in another case, right?
Wouldn't it better to just check if it is a list?

@marcharper
Copy link
Member

Maybe List[List[Any]] is best unless int can check as a subtype of float (I'm not if that's how the numeric type hierarchy in Python works but I doubt it), or Union[int, float] which should accept either.

@marcharper
Copy link
Member

Looks like you have some syntax errors -- check the build errors or try to import the library locally in a python interpreter.

@janga1997
Copy link
Member Author

@marcharper The error has been showing up since the first commit on this PR, which was quite simple.
The only thing I changed was an
import axelrod

and set result_set: axelrod.result_set.ResultSetFromFile

I am assuming it has something to do with the way I import?

@janga1997
Copy link
Member Author

@marcharper Never mind. It is an import error.
It says, ImportError: Failed to import test module: axelrod
, and then AttributeError: module 'axelrod' has no attribute 'result_set'

I saw one of your commits on type hinting, you did something to the effect of from .result_set import ResultSet .
Why does that work ?

@marcharper
Copy link
Member

marcharper commented Feb 1, 2017

The reason that axelrod.results_set.ResultSet doesn't work is because we directly import ResultSet in the top level __init__.py for the library. Instead you would need axelrod.ResultSet.

See PEP328 for info on absolute versus relative imports (or Google around). That's what's going on with from .result_set import ResultSet.

@janga1997
Copy link
Member Author

janga1997 commented Feb 1, 2017

@drvinceknight @marcharper I've exhausted all the options. I've tried different ways to import ResultSet, and none of them passed, even though I am doing it the same way as in other files.
from axelrod import Actions
from .game import Game is how we do it in interaction_utils. I've tried both of them in plot, and none worked. Am I missing something silly?

And is there a way to run tests locally? The commit history on this PR is ugly.

@drvinceknight
Copy link
Member

And is there a way to run tests locally? The commit history on this PR is ugly.

Yup, it's easy enough to run the tests locally, there's information about it here http://axelrod.readthedocs.io/en/latest/tutorials/contributing/strategy/running_tests.html but here's how you would run the tests on the plot.py file:

$ python -m unittest axelrod.tests.unit.test_plot

I've just done that locally and gotten the following error:

Axelrod(janga1997-typeHint-plot) ✗: python -m unittest axelrod.tests.unit.test_plot
Traceback (most recent call last):
  File "/home/vince/anaconda3/lib/python3.5/runpy.py", line 184, in _run_module_as_main
    "__main__", mod_spec)
  File "/home/vince/anaconda3/lib/python3.5/runpy.py", line 85, in _run_code
    exec(code, run_globals)
  File "/home/vince/anaconda3/lib/python3.5/unittest/__main__.py", line 18, in <module>
    main(module=None)
  File "/home/vince/anaconda3/lib/python3.5/unittest/main.py", line 93, in __init__
    self.parseArgs(argv)
  File "/home/vince/anaconda3/lib/python3.5/unittest/main.py", line 140, in parseArgs
    self.createTests()
  File "/home/vince/anaconda3/lib/python3.5/unittest/main.py", line 147, in createTests
    self.module)
  File "/home/vince/anaconda3/lib/python3.5/unittest/loader.py", line 219, in loadTestsFromNames
    suites = [self.loadTestsFromName(name, module) for name in names]
  File "/home/vince/anaconda3/lib/python3.5/unittest/loader.py", line 219, in <listcomp>
    suites = [self.loadTestsFromName(name, module) for name in names]
  File "/home/vince/anaconda3/lib/python3.5/unittest/loader.py", line 153, in loadTestsFromName
    module = __import__(module_name)
  File "/home/vince/src/Axelrod/axelrod/__init__.py", line 11, in <module>
    from .plot import Plot
  File "/home/vince/src/Axelrod/axelrod/plot.py", line 23, in <module>
    axType = matplotlib.axes._subplots.AxesSubplot
AttributeError: module 'matplotlib.axes._subplots' has no attribute 'AxesSubplot'

Which indicates that axType = matplotlib.axes._subplots.AxesSubplot isn't the correct type.

@janga1997
Copy link
Member Author

@drvinceknight I removed the AxesSubplot, and it passed the tests, but that doesn't seem right.
Because we do _, ax = plt.subplots(), which means that ax is the second element in the tuple plt.subplots().
I checked type(plt.subplots()[1]) and it returned matplotlib.axes._subplots.AxesSubplot, which is what I did. Where am I going wrong?

@drvinceknight
Copy link
Member

@drvinceknight I removed the AxesSubplot, and it passed the tests, but that doesn't seem right.
Because we do _, ax = plt.subplots(), which means that ax is the second element in the tuple plt.subplots().
I checked type(plt.subplots()[1]) and it returned matplotlib.axes._subplots.AxesSubplot, which is what I did. Where am I going wrong?

It looks like that's a dynamically created class. Here's a SO question with relevant info: http://stackoverflow.com/questions/11690597/there-is-a-class-matplotlib-axes-axessubplot-but-the-module-matplotlib-axes-has

Try changing it to:

axType = matplotlib.axes.SubplotBase

@drvinceknight
Copy link
Member

Try changing it to:

axType = matplotlib.axes.SubplotBase

when I do that on this branch the tests pass fine but I get a few other errors when running mypy.

@janga1997
Copy link
Member Author

@drvinceknight Can we take this to gitter?

@drvinceknight
Copy link
Member

This looks good to me, one final thing could you add axelrod/plots.py to type_tests.sh please @janga1997?

Copy link
Member

@drvinceknight drvinceknight left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Looks good to me.

@marcharper marcharper merged commit c4aed94 into Axelrod-Python:master Feb 3, 2017
@janga1997 janga1997 deleted the typeHint-plot branch February 3, 2017 04:18
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.

3 participants