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

[MRG] Documentation: added sphinx support and some first docstrings #54

Merged
merged 6 commits into from
Dec 4, 2018

Conversation

NicolasHug
Copy link
Collaborator

This PR adds a doc directory, setups sphinx (separate build and source directories) with sphinx-quickstart and adds some first docstring proposals for the estimators.

Should we set up something on ReadTheDocs or equivalent?

I set up the RTD theme because I like it but I'm open to anything of course.

@codecov
Copy link

codecov bot commented Dec 3, 2018

Codecov Report

Merging #54 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #54   +/-   ##
=======================================
  Coverage   95.39%   95.39%           
=======================================
  Files           9        9           
  Lines         891      891           
=======================================
  Hits          850      850           
  Misses         41       41
Impacted Files Coverage Δ
pygbm/splitting.py 98.95% <ø> (ø) ⬆️
pygbm/gradient_boosting.py 88.06% <ø> (ø) ⬆️
pygbm/binning.py 92.98% <100%> (ø) ⬆️
pygbm/grower.py 92.81% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3334c53...f88ed96. Read the comment docs.

@ogrisel
Copy link
Owner

ogrisel commented Dec 4, 2018

I am fine with readthedoc.

Copy link
Owner

@ogrisel ogrisel left a comment

Choose a reason for hiding this comment

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

+1 for merging quickly and improving the doc and docstring in later PR once it's easy to read the doc online.

@NicolasHug
Copy link
Collaborator Author

Ok I'll add some docstrings for the grower as well, and merge.

Do we want to spin up a Circle CI that build the docs for each PR like in scikit-learn? Unfortunately RTD does not support it at the moment readthedocs/readthedocs.org#1340

@ogrisel
Copy link
Owner

ogrisel commented Dec 4, 2018

I think it's fine just to build a "dev" version of the doc from the master and a "stable" version of the doc from the stable release when we make one.

@NicolasHug
Copy link
Collaborator Author

Ok.

Then I'm not sure what you mean by once it's easy to read the doc online

@NicolasHug NicolasHug merged commit 3f51db2 into ogrisel:master Dec 4, 2018
@NicolasHug NicolasHug deleted the sphinx_setup branch December 4, 2018 21:43
@NicolasHug
Copy link
Collaborator Author

Merged @ogrisel , the docs are here: https://pygbm.readthedocs.io/en/latest/
and the repo RTD project here: https://readthedocs.org/projects/pygbm/

I flattened everything in index.rst but that's just to see what it looks like.

As I don't have admin rights on the github repo, RTD couldn't set up the hooks so new commits won't trigger the doc buildings. I added you as admin on RTD maybe that will fix it.

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.

2 participants