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

Integrate tutorial notebooks in Sphinx documentation #1176

Merged
merged 3 commits into from Oct 21, 2017

Conversation

Projects
None yet
2 participants
@Bultako
Member

Bultako commented Oct 19, 2017

Main modifs are:

  • .gitignore to track the *.ipynb files in docs/folder (these will be copied from GAMMAPY-EXTRA later on)
  • conf.py Sphinx config file in docs to make nbconfig work nicely
  • some *.rst files in docsand some *.py files in gammanysource code folder to replace directive .. code:: by ..code :: pythonand make code inclusion in docs working.
  • You also need to re-install gammany with these new source files.

Remember you need to install nbsphinx:
conda install -c conda-forge nbsphinx

And that's all for the moment.
Just type python setup.py build-docs and this mixed interlinked doc of *rst files and notebooks is generated in one shot. We still have some warnings due to orphans *.ipynb files, but this does not prevent their conversion.

This is the firs step, still work in progress..

TODO

  • Create notebooks.rst with toctree:: directive

Scriptting

  • Copy notebooks from $GAMMAPY_EXTRA to docs/notebooks folder
  • Copy notebooks from $GAMMAPY_EXTRA to _static folder
  • Add header cells with links to raw notebooks in _static folder.
  • Regexp in cells replacing absolute into relative links to doc RST files.
  • Flag only-notebooks
  • Flag exec-noteboooks?

@cdeil cdeil self-assigned this Oct 19, 2017

@cdeil cdeil added this to the 0.7 milestone Oct 19, 2017

@cdeil

This comment has been minimized.

Show comment
Hide comment
@cdeil

cdeil Oct 19, 2017

Member

@Bultako - Thanks!

I think for now, it might be easier to script the copy of the files from $GAMMAPY_EXTRA to the docsfolder (if really needed, ifnbsphinxcan't be configured to directly read the notebooks from there) indocs/conf.py, or really in a function in gammapy/utils/docs.pythat you then call fromdocs/conf.py` like here:

from gammapy.utils.docs import gammapy_sphinx_ext_activate

Of course, yes, long term I would like to have an option for python setup.py build_docs that allows executing different steps of the docs build separately: especially to have the notebook part separately. But that's not a requirement for this to get merged. If you want to make it configurable now, you could use an environment variable GAMMAPY_EXECUTE_NBSPHINX and access that via os.environ. It would be nice to keep that part optional, if it's possible to script that way.

Is there some way to run the build to get "verbose" or "debug" output about what sphinx or nbsphinx is doing? I guess at the moment it's not possible to just execute the new nbsphinx part of the build, to debug and understand it?


My suggestion would be that you do this first, to avoid having to have the notebooks in the gammapy repo like you have now. The thing is that having all the large files makes the PR hard to review (diff so large) and means I can't merge (no large files allowed in the Gammapy repo).

The other thing that's needed to make this PR "mergeable" is to squash the merge commits and commits that contain the large notebook files:
https://github.com/gammapy/gammapy/pull/1176/commits

If you're not sure how to do this with git, let me know. This explanation is pretty good:
http://astropy.readthedocs.io/en/stable/development/workflow/development_workflow.html#how-to-squash


One more thing on your TODO list:

Rename index.ipynb -> notebooks.ipynb

I think really it would be better to change index.ipynb to an RST page? It doesn't contain any code, and RST is better for an index / overview page than a notebook, no?
Not sure if this needs to be done in this first PR or if this can be done later.


I'll have a closer look at this tomorrow.

Member

cdeil commented Oct 19, 2017

@Bultako - Thanks!

I think for now, it might be easier to script the copy of the files from $GAMMAPY_EXTRA to the docsfolder (if really needed, ifnbsphinxcan't be configured to directly read the notebooks from there) indocs/conf.py, or really in a function in gammapy/utils/docs.pythat you then call fromdocs/conf.py` like here:

from gammapy.utils.docs import gammapy_sphinx_ext_activate

Of course, yes, long term I would like to have an option for python setup.py build_docs that allows executing different steps of the docs build separately: especially to have the notebook part separately. But that's not a requirement for this to get merged. If you want to make it configurable now, you could use an environment variable GAMMAPY_EXECUTE_NBSPHINX and access that via os.environ. It would be nice to keep that part optional, if it's possible to script that way.

Is there some way to run the build to get "verbose" or "debug" output about what sphinx or nbsphinx is doing? I guess at the moment it's not possible to just execute the new nbsphinx part of the build, to debug and understand it?


My suggestion would be that you do this first, to avoid having to have the notebooks in the gammapy repo like you have now. The thing is that having all the large files makes the PR hard to review (diff so large) and means I can't merge (no large files allowed in the Gammapy repo).

The other thing that's needed to make this PR "mergeable" is to squash the merge commits and commits that contain the large notebook files:
https://github.com/gammapy/gammapy/pull/1176/commits

If you're not sure how to do this with git, let me know. This explanation is pretty good:
http://astropy.readthedocs.io/en/stable/development/workflow/development_workflow.html#how-to-squash


One more thing on your TODO list:

Rename index.ipynb -> notebooks.ipynb

I think really it would be better to change index.ipynb to an RST page? It doesn't contain any code, and RST is better for an index / overview page than a notebook, no?
Not sure if this needs to be done in this first PR or if this can be done later.


I'll have a closer look at this tomorrow.

@cdeil cdeil changed the title from Include notebooks in the documentation building to Integrate tutorial notebooks in Sphinx documentation Oct 20, 2017

@cdeil

This comment has been minimized.

Show comment
Hide comment
@cdeil

cdeil Oct 20, 2017

Member

I've fixed a few more notebook formatting issues in gammapy-extra, and I've fixed the Sphinx docs build in master by adding this missing dependency: 71dd5c2

@Bultako - Could you please add nbsphinx to .travis.yml so that we start to see it running in CI on this pull request (this should fail: https://travis-ci.org/gammapy/gammapy/jobs/290333660), as well as to docs/environment.yml so that it'll work on RTD when we merge the PR?

Locally I see the following warnings from the Sphinx build:
https://gist.github.com/cdeil/23072fba2afeb652b2427bda787e06ee
We need to get rid for all of them - the Sphinx build on travis-ci is configured such that it fails if there are warnings.

Do you know if there is a way to use the line number information in these warnings somehow to find the problematic notebook cell / line that needs to be fixed?

/Users/deil/code/gammapy/docs/notebooks/cta_1dc_introduction.ipynb:9: WARNING: Explicit markup ends without a blank line; unexpected unindent.
Member

cdeil commented Oct 20, 2017

I've fixed a few more notebook formatting issues in gammapy-extra, and I've fixed the Sphinx docs build in master by adding this missing dependency: 71dd5c2

@Bultako - Could you please add nbsphinx to .travis.yml so that we start to see it running in CI on this pull request (this should fail: https://travis-ci.org/gammapy/gammapy/jobs/290333660), as well as to docs/environment.yml so that it'll work on RTD when we merge the PR?

Locally I see the following warnings from the Sphinx build:
https://gist.github.com/cdeil/23072fba2afeb652b2427bda787e06ee
We need to get rid for all of them - the Sphinx build on travis-ci is configured such that it fails if there are warnings.

Do you know if there is a way to use the line number information in these warnings somehow to find the problematic notebook cell / line that needs to be fixed?

/Users/deil/code/gammapy/docs/notebooks/cta_1dc_introduction.ipynb:9: WARNING: Explicit markup ends without a blank line; unexpected unindent.
@cdeil

This comment has been minimized.

Show comment
Hide comment
@cdeil

cdeil Oct 20, 2017

Member

Is the plan that notebooks will be copied to docs/notebooks instead of directly to the docs/_build output folder? If yes, then please add the docs/notebooks folder to the make clean command here:

rm -rf build dist docs/_build docs/api htmlcov MANIFEST gammapy.egg-info .coverage .cache

Member

cdeil commented Oct 20, 2017

Is the plan that notebooks will be copied to docs/notebooks instead of directly to the docs/_build output folder? If yes, then please add the docs/notebooks folder to the make clean command here:

rm -rf build dist docs/_build docs/api htmlcov MANIFEST gammapy.egg-info .coverage .cache

@cdeil

This comment has been minimized.

Show comment
Hide comment
@cdeil

cdeil Oct 20, 2017

Member

My last comment with the warnings at https://gist.github.com/cdeil/23072fba2afeb652b2427bda787e06ee was incorrect: I think the warnings like

/Users/deil/code/gammapy/docs/notebooks/cta_1dc_introduction.ipynb:9: WARNING: Explicit markup ends without a blank line; unexpected unindent.

should be fixed now. I just didn't realise I was running on an old version of the notebooks because make clean and build_docs doesn't do the clean and copy of notebooks from GAMMAPY_EXTRA yet.

Member

cdeil commented Oct 20, 2017

My last comment with the warnings at https://gist.github.com/cdeil/23072fba2afeb652b2427bda787e06ee was incorrect: I think the warnings like

/Users/deil/code/gammapy/docs/notebooks/cta_1dc_introduction.ipynb:9: WARNING: Explicit markup ends without a blank line; unexpected unindent.

should be fixed now. I just didn't realise I was running on an old version of the notebooks because make clean and build_docs doesn't do the clean and copy of notebooks from GAMMAPY_EXTRA yet.

@Bultako

This comment has been minimized.

Show comment
Hide comment
@Bultako

Bultako Oct 20, 2017

Member

@cdeil

  • YAML files and Makefile fixed

I think the plan is still copying notebooks from $GAMMAPY_EXTRA to docs/notebooks.
There are still some warnings related with orphan notebooks (not present in any doctree directive) that may be solved with a cleaned notebooks.rst file, I will check it and commit later.

Member

Bultako commented Oct 20, 2017

@cdeil

  • YAML files and Makefile fixed

I think the plan is still copying notebooks from $GAMMAPY_EXTRA to docs/notebooks.
There are still some warnings related with orphan notebooks (not present in any doctree directive) that may be solved with a cleaned notebooks.rst file, I will check it and commit later.

Bultako added some commits Oct 19, 2017

Initial config and code directive fixed
Generated doc files added just for inspection

Generated html doc removed

Modif YAML files and Makefile clean notebooks

Remove notebooks and examples

Delete gammapy.css.~LOCAL

Update .gitignore
@cdeil

This comment has been minimized.

Show comment
Hide comment
@cdeil

cdeil Oct 20, 2017

Member

Locally I still get these warnings:

Users/deil/code/gammapy/docs/notebooks/cta_1dc_introduction.ipynb:9: WARNING: Explicit markup ends without a blank line; unexpected unindent.
/Users/deil/code/gammapy/docs/notebooks/cta_1dc_introduction.ipynb:1657: WARNING: Explicit markup ends without a blank line; unexpected unindent.
/Users/deil/code/gammapy/docs/notebooks/cta_data_analysis.ipynb:9: WARNING: Explicit markup ends without a blank line; unexpected unindent.
WARNING: /Users/deil/code/gammapy/docs/notebooks/data_fermi_lat.ipynb:: (ERROR/3) Anonymous hyperlink mismatch: 1 references but 0 targets.
See "backrefs" attribute for IDs.

The CI build hasn't started ... the main goal for this PR would be to get green light there:
https://travis-ci.org/gammapy/gammapy/jobs/290437970

@Bultako - Do you have time to work on this PR in the coming days or is this ready to be merged from your side? I would prefer to merge this soon to see if it works on RTD, and to move the task list from above to a follow-up issue / pull request.

Member

cdeil commented Oct 20, 2017

Locally I still get these warnings:

Users/deil/code/gammapy/docs/notebooks/cta_1dc_introduction.ipynb:9: WARNING: Explicit markup ends without a blank line; unexpected unindent.
/Users/deil/code/gammapy/docs/notebooks/cta_1dc_introduction.ipynb:1657: WARNING: Explicit markup ends without a blank line; unexpected unindent.
/Users/deil/code/gammapy/docs/notebooks/cta_data_analysis.ipynb:9: WARNING: Explicit markup ends without a blank line; unexpected unindent.
WARNING: /Users/deil/code/gammapy/docs/notebooks/data_fermi_lat.ipynb:: (ERROR/3) Anonymous hyperlink mismatch: 1 references but 0 targets.
See "backrefs" attribute for IDs.

The CI build hasn't started ... the main goal for this PR would be to get green light there:
https://travis-ci.org/gammapy/gammapy/jobs/290437970

@Bultako - Do you have time to work on this PR in the coming days or is this ready to be merged from your side? I would prefer to merge this soon to see if it works on RTD, and to move the task list from above to a follow-up issue / pull request.

@Bultako

This comment has been minimized.

Show comment
Hide comment
@Bultako

Bultako Oct 20, 2017

Member

@cdeil

I think we were working in different versions of gammapy-extra

  • I have pulled the last master version gammapy-extra 8359164
  • I have done a make clean in gammapy root folder
  • I have done a python setup.py build_docs and got a warning for data_fermi_lat.ipynb

There is a broken link in that nb, I have fixed and rebuild the docs, this time I had no warnings..
I have made a PR in gammapy-extra to fix this.
gammapy/gammapy-extra#89

Re: Travis CI green light.
It blocked because of IPython needed for syntax highlighting in notebooks. I have edited the YAML files. Crossing fingers...

We can merge this PR as it is when we'll have green light, next week I'll be a bit busy.

Member

Bultako commented Oct 20, 2017

@cdeil

I think we were working in different versions of gammapy-extra

  • I have pulled the last master version gammapy-extra 8359164
  • I have done a make clean in gammapy root folder
  • I have done a python setup.py build_docs and got a warning for data_fermi_lat.ipynb

There is a broken link in that nb, I have fixed and rebuild the docs, this time I had no warnings..
I have made a PR in gammapy-extra to fix this.
gammapy/gammapy-extra#89

Re: Travis CI green light.
It blocked because of IPython needed for syntax highlighting in notebooks. I have edited the YAML files. Crossing fingers...

We can merge this PR as it is when we'll have green light, next week I'll be a bit busy.

@cdeil

This comment has been minimized.

Show comment
Hide comment
@cdeil

cdeil Oct 21, 2017

Member

Merging now. I want to do some more changes on this, but let's see if this is working on RTD at all.

Member

cdeil commented Oct 21, 2017

Merging now. I want to do some more changes on this, but let's see if this is working on RTD at all.

@cdeil

cdeil approved these changes Oct 21, 2017

@cdeil cdeil merged commit 0d475fc into gammapy:master Oct 21, 2017

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
@cdeil

This comment has been minimized.

Show comment
Hide comment
@cdeil

cdeil Oct 21, 2017

Member

It worked! 🎉
@Bultako - Thank you!

The build log is at http://readthedocs.org/projects/gammapy/builds/6156543/, and the timeout is a weird bug in the RTD docker infrastructure - all of our builds end in a timeout, even if it was successful. So for now one has to look through the log and see if there's any error, which in this case there isn't.

Member

cdeil commented Oct 21, 2017

It worked! 🎉
@Bultako - Thank you!

The build log is at http://readthedocs.org/projects/gammapy/builds/6156543/, and the timeout is a weird bug in the RTD docker infrastructure - all of our builds end in a timeout, even if it was successful. So for now one has to look through the log and see if there's any error, which in this case there isn't.

@Bultako

This comment has been minimized.

Show comment
Hide comment
@Bultako

Bultako Oct 23, 2017

Member

good :)
I see in http://docs.gammapy.org the search does not find any notebook, but locally in my html built docs the search works pretty well. do you have an idea on how to solve this? should I open an issue an have a look?

Member

Bultako commented Oct 23, 2017

good :)
I see in http://docs.gammapy.org the search does not find any notebook, but locally in my html built docs the search works pretty well. do you have an idea on how to solve this? should I open an issue an have a look?

@Bultako Bultako deleted the Bultako:docbuilding-notebooks branch Oct 23, 2017

@cdeil

This comment has been minimized.

Show comment
Hide comment
@cdeil

cdeil Oct 23, 2017

Member

I tried this and it finds an entry:
http://docs.gammapy.org/en/latest/search.html?q=WMAP&check_keywords=yes&area=default
but it doesn't display correctly in the search results.

Do you have an example search URL you're using to test?

Let me try and wipe RTD to trigger a clean build now and see if that makes it better.

Member

cdeil commented Oct 23, 2017

I tried this and it finds an entry:
http://docs.gammapy.org/en/latest/search.html?q=WMAP&check_keywords=yes&area=default
but it doesn't display correctly in the search results.

Do you have an example search URL you're using to test?

Let me try and wipe RTD to trigger a clean build now and see if that makes it better.

@cdeil

This comment has been minimized.

Show comment
Hide comment
@cdeil

cdeil Oct 23, 2017

Member

I've wiped RTD and triggered a new build here: https://readthedocs.org/projects/gammapy/builds/6162287/

@Bultako - Do you have an RTD account? If yes, I would be happy to give you admin permissions there for Gammapy.

Member

cdeil commented Oct 23, 2017

I've wiped RTD and triggered a new build here: https://readthedocs.org/projects/gammapy/builds/6162287/

@Bultako - Do you have an RTD account? If yes, I would be happy to give you admin permissions there for Gammapy.

@Bultako

This comment has been minimized.

Show comment
Hide comment
@Bultako

Bultako Oct 23, 2017

Member

@cdeil
http://docs.gammapy.org/en/latest/search.html?q=data+challenge
does work nicely locally but does not find the notebooks in RTD

I've just created my bultako account in RTD :)

Member

Bultako commented Oct 23, 2017

@cdeil
http://docs.gammapy.org/en/latest/search.html?q=data+challenge
does work nicely locally but does not find the notebooks in RTD

I've just created my bultako account in RTD :)

@cdeil

This comment has been minimized.

Show comment
Hide comment
@cdeil

cdeil Oct 23, 2017

Member

@Bultako - You're now an admin on RTD for Gammapy.

We've had problems before with Sphinx search not showing certain things: #611

Maybe you could open a new issue in the Gammapy tracker? This one is closed and I don't have time to investigate today.

Member

cdeil commented Oct 23, 2017

@Bultako - You're now an admin on RTD for Gammapy.

We've had problems before with Sphinx search not showing certain things: #611

Maybe you could open a new issue in the Gammapy tracker? This one is closed and I don't have time to investigate today.

@Bultako

This comment has been minimized.

Show comment
Hide comment
@Bultako

Bultako Oct 23, 2017

Member

@cdeil

After some investigation I've found RTD always build an HTML version together with a JSON version of the docs. These are used for web serving & search indexing, respectively.
http://docs.readthedocs.io/en/latest/yaml-config.html?highlight=JSON

But, our last builds in RTD have not finished properly, so I suspect JSON format has not been built properly, as it is also the case with htmlzip, epub and pdf formats. Note that the latest version of the RTD documentation does not link properly to these formats
e.g. https://media.readthedocs.org/pdf/gammapy/latest/gammapy.pdf

You said you could have look at the build logs in RTD?
I have not found how to do this..

Member

Bultako commented Oct 23, 2017

@cdeil

After some investigation I've found RTD always build an HTML version together with a JSON version of the docs. These are used for web serving & search indexing, respectively.
http://docs.readthedocs.io/en/latest/yaml-config.html?highlight=JSON

But, our last builds in RTD have not finished properly, so I suspect JSON format has not been built properly, as it is also the case with htmlzip, epub and pdf formats. Note that the latest version of the RTD documentation does not link properly to these formats
e.g. https://media.readthedocs.org/pdf/gammapy/latest/gammapy.pdf

You said you could have look at the build logs in RTD?
I have not found how to do this..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment