josejimenezluna / pyGPGO

Bayesian optimization for Python
http://pygpgo.readthedocs.io
MIT License
241 stars 62 forks source link

[JOSS]: More prose & clearer examples in online docs #7

Closed dfm closed 7 years ago

dfm commented 7 years ago

I'm reviewing the JOSS submission at openjournals/joss-reviews#41 and most things are looking good, but I'll open a few issues here.

To be consistent with the JOSS requirements, the docs are going to need some work. At the moment, the documentation has a brief statement of need, a single example, and then API docs. More discussion should be included on the readthedocs page to make it easier to get started. This should include things like one or two tutorials (the examples directory is not sufficient - these examples don't even include any comments!), a page outlining the installation procedure, and a more detailed description of the available options.

Another thing that might be good to include is a comparison to some of the other existing Bayesian optimization packages that are already available in Python: fmfn/BayesianOptimization, GPyOpt, and skopt to name a few.

josejimenezluna commented 7 years ago

Hi @dfm

Thanks for your useful comments. Several improvements have been made:

Let me know if something else needs to be addressed.

dfm commented 7 years ago

This looks great! Two comments:

  1. Can you make the documentation URL more obvious on github? You can add it to the header by clicking on the edit button like the one in the image below (that's one of my repos because I don't get the edit button for your repos :wink:). And I would also make it more prominent in the README.
screen shot 2017-10-20 at 2 37 41 pm
  1. Most of the internal reference links are broken in your docs right now. Check the syntax and use something like :class:`pyGPGO.surrogates.GaussianProcess ` instead of the <pyGPGO.surrogates.GaussianProcess> that you're using now (e.g. http://pygpgo.readthedocs.io/en/latest/features.html#surrogate-models-pygpgo-surrogates)
josejimenezluna commented 7 years ago

Hi @dfm !

Let me know again if further changes are required.

dfm commented 7 years ago

Looks great! One last issue with the links. I think that you're still using the markdown style for citations, but it would be good to switch to the correct RST syntax: http://docutils.sourceforge.net/docs/user/rst/quickref.html#citations

josejimenezluna commented 7 years ago

Thanks for noticing. The correct RST citation syntax is used in the docs.

josejimenezluna commented 7 years ago

Hi @dfm!

Is there something else that needs to be addressed or can I close this? :)

dfm commented 7 years ago

Oops. Nope. Looks good now!