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 sample game generators from bimatrix-generators #392

Merged
merged 8 commits into from
Mar 18, 2018

Conversation

oyamad
Copy link
Member

@oyamad oyamad commented Mar 7, 2018

For the background, see QuantEcon/GameTheory.jl#53 (comment). Cc: @ptigwe

  • Generate doc files

@oyamad oyamad added the in-work label Mar 7, 2018
@oyamad
Copy link
Member Author

oyamad commented Mar 7, 2018

@mmcky Running python qe_apidoc.py does not generate doc files for bimatrix_generators.py which is located in a subdirectory game_generators in game_theory. Could you give an instruction, to me or to @QBatista, about how to modify qe_apidoc.py?

@coveralls
Copy link

coveralls commented Mar 7, 2018

Coverage Status

Coverage increased (+0.2%) to 95.136% when pulling 0dfa4b7 on oyamad:bimatrix-generators into 49ad385 on QuantEcon:master.

@mmcky
Copy link
Contributor

mmcky commented Mar 8, 2018

Thanks @oyamad. @QBatista we can work on this tomorrow, and I will update on how qe_apidoc.py is put together. I haven't documented it well as I was hoping to make it automated (to prevent things not being included) using autodoc.

@mmcky
Copy link
Contributor

mmcky commented Mar 9, 2018

The issue is that our qe_apidoc.py only implements one level deep and the directory tree is not recursively searched. To get this PR merged we can add this sub-module into qe_apidoc.py to get the documentation to build.

@oyamad
Copy link
Member Author

oyamad commented Mar 9, 2018

If we add something like

game_theory_files += glob("../quantecon/game_theory/[!tests]*/[a-z0-9]*.py")

to

game_theory_files = glob("../quantecon/game_theory/[a-z0-9]*.py")

would that work?

@mmcky
Copy link
Contributor

mmcky commented Mar 9, 2018

That is essentially what needs to be done - except that would list the generators under the game_theory section and not as another sub-directory within game_theory.

@QBatista and I can work on this while he is here. It might be nice to improve the code to make this sort of adjustment easier to do. I am thinking of a simpler json config file that would determine the directory structure and then get python to build the directories and documents from that config file.

@oyamad
Copy link
Member Author

oyamad commented Mar 9, 2018

I see. At the moment it is fine with me: the functions in game_theory/game_generators/bimatrix_generators.py are imported into the qe.game_theory submodule (i.e., they can be called by qe.game_theory.*).

@mmcky
Copy link
Contributor

mmcky commented Mar 9, 2018

I see. In that case:

game_theory_files += glob("../quantecon/game_theory/game_generators/[a-z0-9]*.py")

should work.

@mmcky
Copy link
Contributor

mmcky commented Mar 9, 2018

It's also OK if you want to use your pattern to capture future sub-directories also if you think the sub-modules will always be available at the top level of the game_theory module.

@oyamad
Copy link
Member Author

oyamad commented Mar 9, 2018

make html does not work with this approach: ModuleNotFoundError: No module named 'quantecon.game_theory.bimatrix_generators'

@mmcky
Copy link
Contributor

mmcky commented Mar 14, 2018

@oyamad this isn't elegant but includes the game_generators subdirectory into the qe_apidoc.py tool. I would like to rewrite this to be recursive and search the directory tree automatically but this manually adds that directory and then adds the bimatrix_generators module to the game_theory toc to keep the toc flat.

image

As this branch sits in your fork I don't think I can pre-render using rtd (as a version) for you to have a look at but it does render locally (assuming you have your version of quantecon installed or is active in an environment).

@oyamad
Copy link
Member Author

oyamad commented Mar 15, 2018

@mmcky Great, thanks so much! It renders perfectly locally, although there is some warning with make html:

:1: (WARNING/2) Inline interpreted text or phrase reference start-string without end-string.

(Does not seem to affect the outcome.)

@mmcky
Copy link
Contributor

mmcky commented Mar 15, 2018

Hi @oyamad - Great. I am comfortable with the warning from sphinx for now and happy for this to be merged when you have finalised development.

@oyamad
Copy link
Member Author

oyamad commented Mar 15, 2018

@oyamad
Copy link
Member Author

oyamad commented Mar 15, 2018

This is ready to be merged.

@oyamad oyamad added ready and removed in-work labels Mar 15, 2018
@mmcky mmcky merged commit 051ba68 into QuantEcon:master Mar 18, 2018
@oyamad
Copy link
Member Author

oyamad commented Mar 19, 2018

Many thanks @QBatista @shizejin @mmcky !

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

Successfully merging this pull request may close these issues.

None yet

5 participants