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

SPHINX based html docs #418

Merged
merged 7 commits into from
May 15, 2023
Merged

Conversation

bwheelz36
Copy link
Collaborator

Hi @fmfn and @till-m

As discussed, I drafted a html based documentation.
All source code is in the 'docsrc' directory.
To build the docs, navigate to this directory, install the requirements listed in the requirements file and do make github.
This will generate a folder called docs. In this folder, open index.html in a browser to review the documents.

The documents are made of two things:

  1. the notebook files inside examples (defined in docsrc/examples.rst).
  2. Autodocs generated from scanning the code docstrings (docsrc/code_docs.rst)

I have also added a workflow that will build these docs, and eventually publish them to new branch (.github/workflows/build_docs.yml). We can then use github pages to host the docs directly from this branch. However @fmfn will have to implement a lot of these steps as they require admin rights - but we can do that when we get to it. For now let me know what you think of the draft html.

Future documents can be written in notebooks, markdown, or rst - and then incorporated into the html.

@bwheelz36 bwheelz36 requested review from fmfn and till-m May 7, 2023 23:54
@codecov-commenter
Copy link

codecov-commenter commented May 7, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (33b99ec) 98.57% compared to head (917b49a) 98.57%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #418   +/-   ##
=======================================
  Coverage   98.57%   98.57%           
=======================================
  Files           8        8           
  Lines         560      560           
  Branches       79       79           
=======================================
  Hits          552      552           
  Misses          4        4           
  Partials        4        4           

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@till-m
Copy link
Member

till-m commented May 8, 2023

Generally this is looking really good imo!

Two small things I noticed is that a) you're listed as copyright holder of the package in the footer and b) in the constrained optimization notebook, there seems to be a problem with importing matplotlib. The following cells fail as a result.

Are the notebooks re-rendered on commit/new PR?

@bwheelz36
Copy link
Collaborator Author

  1. I think it means the docs are copyright, but I'm happy to change that to Fernando. I don't really know what this means anyway since the code and repo is governened by the license under which it is released. I might just try to release this copy right line.
  2. "Are the notebooks re-rendered on commit/new PR?" no - as per my comment on Add run notebooks test #417, this was just an error on my end.

Copy link
Member

@till-m till-m left a comment

Choose a reason for hiding this comment

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

Looks good to me. Similar to #417, is it necessary to change the notebooks in this PR?
I also noticed that the changes in #417 seem to be covered by this PR, too.
Is this intentional? Does this PR need more time and we should merge #417 in the meantime?

@bwheelz36
Copy link
Collaborator Author

bwheelz36 commented May 9, 2023

  • this was branched from Add run notebooks test #417 - is that bad practice? my intention was to make the merges easier
  • happy to leave this as a draft until we confirm @fmfn is keen as he will have to do some of the setup anyway.
  • I should clarify - I actually didn't change the notebooks per-se, I just re-ran them. I shouldn't have saved the result but old habits. notebooks are nice for some things but a nightmare with git...

@till-m
Copy link
Member

till-m commented May 10, 2023

is that bad practice?

I don't think it is, I was just checking whether you had some bigger strategy for getting this set up

@bwheelz36 bwheelz36 marked this pull request as draft May 11, 2023 07:50
@bwheelz36 bwheelz36 marked this pull request as ready for review May 15, 2023 07:39
@bwheelz36 bwheelz36 merged commit 434ac43 into bayesian-optimization:master May 15, 2023
5 checks passed
@bwheelz36 bwheelz36 deleted the html_docs branch May 15, 2023 07:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants