Closed pdeiml closed 6 years ago
@pdeiml - I've improved a few things in fed65041b8b70e5551be1e2c98ebda7efd77f1d3
Before this can be merged, the two-step build process needs to be sorted out, as you say.
Locally I think running make.py webpage; cd documentation; make html
is fine, although of course the cd documentation; make html
part could also be run via make.py
for convenience.
The question is how to handle the case on RTD. What they execute is described here: http://docs.readthedocs.io/en/latest/builds.html#understanding-what-s-going-on
They execute setup.py
if present (we don't have one) followed by the sphinx build which executes documentation/conf.py
. So there's a few options. We could do something like this (https://github.com/gammapy/gammapy/blob/11b745afa2d559ad485b4d47de711f65edaeeefe/docs/conf.py#L173), i.e. execute ../make.py all
from documentation/conf.py
if we are on RTD. Or we could add a setup.py
which calls make.py all
. Both of those would work, but they aren't very nice, because in both cases RTD does something different than what we do locally, i.e. it's harder to debug and maintain compared to if the same thing were done on RTD and locally and on travis-ci.
But I don't think there's a simple solution here. I'll meditate on this some more, and then implement a solution here tomorrow.
What about the following solution:
RTD executes the conf.py script and there we add "make.py webpage" instead of "make.py all". For the RTD webpage you do not have to run everything of make but only the part of the webpage. Additionally, we delete https://github.com/gammapy/gamma-cat/blob/master/make.py#L137 in order to not run the webpage building when executing "make.py all".
With this, the RTD sphinx building process should perfectly work (I don't know how to check this without committing to master). Testing the webpage locally should then work with "cd documentation && make html", shouldn't it?
And additionally, when doing some other changes on the repo which do not touch the webpage, you can run "make.py all" at the end without running into the problem of building rat files which should not be part of the repo.
The webpage build usually needs all other steps to be up to date, i.e. output and index files to be present, so I would suggest we run make.py all --clean --webpage
on RTD.
I added flags to make.py all
just now, with the default being that it runs collection, catalog and checks, and then clean and webpage are off by default.
$ time ./make.py all --help
Usage: make.py all [OPTIONS]
Run all steps.
Options:
--clean Run clean step?
--no-collection Skip collection step?
--no-catalog Skip catalog step?
--no-checks Skip checks step?
--webpage Run webpage step?
--help Show this message and exit.
without running into the problem of building rat files which should not be part of the repo.
I've added documentation/data
to .gitignore
and to make.py clean
, since it's now mostly a folder where we auto-generate files that shouldn't be committed.
I've added the make.py
call in documentation/conf.py
in f356c228 .
Merging this now. Let's see what happens on RTD. Will probably require a few follow-up commits in master to get it to work.
At https://readthedocs.org/projects/gamma-cat/builds/6273028/ I see this error:
/bin/sh: 1: time: not found
Executing: cd .. && time ./make.py all --clean --webpage
Let's try this: ebceb98c
Looks like it worked: http://gamma-cat.readthedocs.io/data/source_list.html http://gamma-cat.readthedocs.io/data/sources/source_37.html
I don't know why the build says error at the top: https://readthedocs.org/projects/gamma-cat/builds/6273153/ (the way RTD shows the build log is terrible)
Looking into the individual sections running sphinx-build
I see that the make.py all
is executed several times. This is a waste of time. I'm turning off the PDF and ePUB build on RTD now.
So @pdeiml - Now you could start another PR to flesh out the source list and / or source detail pages to contain the info we have now here: https://gammapy.github.io/gamma-cat/sources.html I would suggest you do that and limit the PR to that first, because when that "feature" is on the new webpage, the old HTML and JS files can be removed, as discussed in #171
Hi,
I found two bugs corresponding to this PR.
1) I did locally "python make.py webpage" and afterwards a "make html" in documentation. Unfortunately, the section GAMMA CAT DATA -> Sources with the complete source list does not occur when I open the webpage locally. Am I doing anything wrong?
2) On RTD the links to the datasets does not work, e.g. http://gamma-cat.readthedocs.io/data/sources/source_21.html contains a link to a sed file but you can try to click on it and it will not show up.
./make.py all --clean --webpage
. Or carefully read the log from ./make.py webpage
and look at the files and see what's wrong. Also make sure you're on the right branch (master should work).For completeness:
It works locally by executing the following commands:
python make.py clean python make.py webpage cd documentation && make html
I accidentally closed https://github.com/gammapy/gamma-cat/pull/170, hence, I opened this new one here. It is basically exactly the same as https://github.com/gammapy/gamma-cat/pull/170 but I kept out the restructuring of the documentation folder and I will create a issue about this, I think.
Only the template rst files are being in the repository, the rst-files generated with "python make.py webpage" are NOT added. Therefore, you have to approve this PR by adding this make.py webpage command to https://github.com/gammapy/gamma-cat/blob/master/documentation/conf.py. Should we change the "all" option in make.py in such a way that "webpage" is not executed, because it is later on executed by "make html" (see above) and then is would be executed two times. Please note that "python make.py webpage" will break if there is no folder documentation/data/sources in which the source rst files are written to (see error in travis CI). Therefore, you may have to add something like "mkdir documentation/data/sources" to conf.py (or to webpage.py).
If this is done and make.py webpage still works. This PR is ready to merge.