search

AngryMaciek / angry-moran-simulator

A general game-theoretical framework to carry out scientific simulations of populations' evolution according to the Moran model 🧮
https://angrymaciek.github.io/angry-moran-simulator/
MIT License
6 stars 5 forks source link

[JOSS REVIEW] Suggestions for Documentation #5

Closed xin-huang closed 4 years ago

xin-huang commented 4 years ago

This is a part of the JOSS review (https://github.com/openjournals/joss-reviews/issues/2643)

  1. I found a Jupyter notebook https://github.com/AngryMaciek/angry-moran-simulator/blob/master/tests/usecase.ipynb could reproduce the examples. I suggest the authors provide a link to this notebook in their documentation explicitly, so that users can quickly replicate the examples.
  2. After installing the package successfully in my computer with Ubuntu 18.04, I failed to replicate the examples in https://github.com/AngryMaciek/angry-moran-simulator/blob/master/tests/usecase.ipynb at first. I have to manually install jupyter, pyyaml and seaborn under the virtual environment moranpycess before successfully running the notebook. Thus, I suggest the authors add jupyter, pyyaml and seaborn into https://github.com/AngryMaciek/angry-moran-simulator/blob/master/env/main.yml, so that users can easily reproduce the examples.
  3. As the checklist suggests, a statement of need is required in the documentation. Besides, the authors may provide a link to https://github.com/AngryMaciek/angry-moran-simulator/blob/master/env/main.yml, so that users can find dependencies for this package easily.
  4. Typos in https://github.com/AngryMaciek/angry-moran-simulator/blob/master/documentation.md
    • The word "sygnature" should be "signature"
AngryMaciek commented 4 years ago

(4) should be fixed in: https://github.com/AngryMaciek/angry-moran-simulator/commit/c3b2f6aeb595d5f06575b81588f7da869835eedf

AngryMaciek commented 4 years ago

(1) is fixed from 0fe50fb forward, also added a small Table Of Contents in the documentation.

AngryMaciek commented 4 years ago

(2) is fixed from: 1f1a09b forward; pyyaml is not necessary so I removed the import statement. Please note that the environment will further change according to the suggestions in #7 .

AngryMaciek commented 4 years ago

(3) is addressed in 71b6216: added two sections to the documentation file.

@xin-huang : Thank you for the comments! I believe I have addresses all of the suggestions above.

xin-huang commented 4 years ago

Hi @AngryMaciek, thank you for addressing my concerns. All the changes look good.