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

Adding Sphinx documentation #197

Closed
DougBurke opened this issue Mar 25, 2016 · 56 comments
Closed

Adding Sphinx documentation #197

DougBurke opened this issue Mar 25, 2016 · 56 comments

Comments

@DougBurke
Copy link
Contributor

DougBurke commented Mar 25, 2016

Demo

I have a not-guaranteed-to-be-up-to-date version of the documentation at http://hea-www.harvard.edu/~dburke/playground/sherpa/index.html - this should not be taken as an official product as it is, in large part, an experiment. It may go away at any time!

Background

As I know there's been discussion about this at PyAstro16, I thought it worth mentioning what I've learnt in my investigations to help anyone who wants to work on this. Please do!

There is a branch at feature/sphinx-docs which shows what I've done. I have a version on read-the-docs: output and configuration information.

From memory, the issues include (they are not ordered in severity, can overlap somewhat, and may actually feed into other possible improvements we could do):

  • Move the contents of the existing doc/ directory somewhere else (e.g. notebook/) so that doc/ can be used for Sphinx.

  • I had trouble getting Sherpa to build on Read The Docs as soon as I tried getting it to run code (e.g. to make sure the screen output is up to date); I forget the details but the last log suggests I gave up as soon as it complained about missing yacc. My thoughts at the time are that the build would have to be done locally, with the results pushed to the web site, but I didn't investigate this any further. There have been a number of build improvements to Sherpa since then, but we still need to compile code.

  • I needed to add a customized IPython directive to grab the output from the Sherpa logger and include it into the Sphinx shell. It's pretty basic, but to save time re-inventing the wheel you can find it at 676cd6f. If you don't do this then a lot of screen output is not included in the HTML output, which defeats the purpose just a little bit ;-)

    The version has since been updated (to address issues I found). It is less important when using the
    OO API, since most of the use of the logging infrastructure comes from the UI layer, but there are
    still places where it is used (e.g. the error analysis, although there I need to turn off the multi-cpu support for it to work properly, and I've yet to investigate why). [note added Fri May 6 10:54:23 EDT 2016]

  • The presence of optional/multiple code paths is a challenge, both for building (what is included and what isn't) and actual documentation text: for example how to best explain functionality that may only be present in one I/O backend. This is a combination of issues: technological, documentation content, and social (what do we want).

  • If you auto-generate the module layout then you end up with a confusing mess. Many Sherpa modules re-export content from other modules, which leads to a lot of repeated information and some very-large modules (e.g. sherpa.astro.ui), both of which lead to reader confusion and fatigue. If you don't autogenerate the layout, then you have a lot of work ahead of you, and you are still likely to hit some of these problems.

  • There has been some clean up of the code since I worked on this, removing unused and CIAO-specific code, but there are probably still files left over that need to be excluded from the auto-generated documentation. Also, some of the modules are too large (I know I've already mentioned this, but it needs repeating).

  • I am fairly sure some of the "clever" wrapping we do, to make some functionality easier to use from IPython, interacts poorly with Python docstrings, and hence the generated documentation. As an example, we allow users to say sherpa.astro.ui.set_source(sherpa.astro.ui.polynom1d.cpt) but sherpa.astro.ui.polynom1d is actually a sherpa.ui.utils.ModelWrapper instance around the sherpa.models.basic.Polynom1D class, which is where the docstrings should be (but currently are). Also, Polynom1D is also available as sherpa.models.Polynom1D (just re-iterating the point about repeated documentation). We (@olaurino and I) have talked about this particular issue before (i.e. pass through the model docstring) but that hasn't been worked on (and the use case of making sure that the information is available to Sphinx wasn't thought of).

    I keep on confusing myself about this; the problem is not for the object that was created, but for the class that creates the model object. That is, help(sherpa.ui.polynom1d) returns information on the ModelWrapper class. [note clarified Wed May 11 12:02:38 EDT 2016]

  • I think we have good coverage of the UI API (although some is still missing and the content can be improved), except for a few specific areas such as the model instances and functions written in C, but the documentation of the lower-level routines is in a sorry state. Work on Sphinx documentation should help illuminate what areas need work, and can be done outside of this project (I have been trying to add stuff as I work on code fixes and improvements, in particular the C functions, but lots more to be done).

  • See also my comment on #22 which discusses a problem I had with the DS9-related code when running Sphinx
    [note added 16:25 EDT Friday 25 March 2016]

  • One suggestion that has been made is to not include the session-based API (what is referred to as the UI layer above - i.e. sherpa.astro.ui and sherpa.ui) in the Sphinx documentation. This would essentially have the UI layer documented as part of CIAO and the object-oriented API (which needs a lot of documentation) for the Sphinx documentation.
    [note added 14:48 EDT Tuesday 26 April 2016]

@cdeil
Copy link
Contributor

cdeil commented Mar 25, 2016

@DougBurke - Thanks for working on this!

FYI, in case you hadn't seen this: readthedocs now supports conda install for dependencies:
http://read-the-docs.readthedocs.org/en/latest/conda.html

I haven't switched to conda on RTD for my projects, but I think sunpy and several other have, and they are happy and could probably help if there are any questions / issues.

@DougBurke
Copy link
Contributor Author

I guess this let's us get docs for released versions (if you can install
from arbitrary conda channels), but for the development version we'd still
need to build the C code. I think. Or re-architect the sherpa build to make
those parts optional, but there are issues with that approach if we want to
run code.

Although I'm currently at lunch with the kids so an distracted!

On Fri, Mar 25, 2016, 12:03 Christoph Deil notifications@github.com wrote:

@DougBurke https://github.com/DougBurke - Thanks for working on this!

FYI, in case you hadn't seen this: readthedocs now supports conda install
for dependencies:
http://read-the-docs.readthedocs.org/en/latest/conda.html

I haven't switched to conda on RTD for my projects, but I think sunpy and
several other have, and they are happy and could probably help if there are
any questions / issues.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#197 (comment)

@cdeil
Copy link
Contributor

cdeil commented Mar 25, 2016

for the development version we'd still need to build the C code

I thought this works with Sherpa, i.e. one could conda install sherpa to get numpy, matplotlib, astropy and then with that Python setup.py install Sherpa if one has the system gcc.
But maybe it's not the way to go for Sherpa, I don't know.

@olaurino
Copy link
Member

fftw3 is needed for supporting some basic convolution features, while the region library from CIAO is needed to build the region extension. It should be easy enough to make the region extension optional, but there would still be an issue with fftw3. A partial installation, however, would lead to partial documentation as well.

I don't really understand why RTD duplicates the effort required to build the project rather than accepting an upload as services like coveralls do. Then one could simply add a snippet of configuration on travis to make the docs and upload them to RTD. But maybe that's already possible and I am not aware of it?

@DougBurke
Copy link
Contributor Author

@olaurino I'm pretty sure you can upload completed docs to RTD. It is just
a lot more convenient of RTD can do it, so you can use github hooks to pick
up each build. I didn't investigate this area much so there could be many
options here that I don't know about.

On Fri, Mar 25, 2016, 13:21 Omar Laurino notifications@github.com wrote:

fftw3 is needed for supporting some basic convolution features, while the
region library from CIAO is needed to build the region extension. It should
be easy enough to make the region extension optional, but there would still
be an issue with fftw3. A partial installation, however, would lead to
partial documentation as well.

I don't really understand why RTD duplicates the effort required to build
the project rather than accepting an upload as services like coveralls do.
Then one could simply add a snippet of configuration on travis to make the
docs and upload them to RTD. But maybe that's already possible and I am not
aware of it?


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#197 (comment)

@olaurino
Copy link
Member

It is just
a lot more convenient of RTD can do it, so you can use github hooks to pick
up each build.

If it worked a little like coveralls and similar services, Travis would pick up each build and then send the docs as part of the after_success task.

@DougBurke
Copy link
Contributor Author

@olaurino I've rebased this branch against the master branch, since I wanted to pick up the recent changes. As this is only an experimental branch (ie I don't expect to be making any PRs from it), is it okay if I push the rebased branch, or should I start a new branch?

@olaurino
Copy link
Member

@DougBurke even if it wasn't an experimental branch, you could feel free to force-push to the same branch after a rebase, even if it is backing a PR. The only drawback comes if you are working on the same branch with somebody else. In that case things should be coordinated. If a PR was open, before the force-push you can note the old tip of the branch in a comment, so one can track the history of the PR back to the original commits, in case anything goes wrong.

@DougBurke
Copy link
Contributor Author

The rebase has been pushed to GitHub.

@DougBurke
Copy link
Contributor Author

So, it looks like - for the Sphinx build - we can not use IPython versions 4.1 and 4.2 due to
ipython/ipython#8733 - as I make liberal use of @savefig.

My current set up is:

% conda create -n=sherpa-sphinx python=2.7 matplotlib astropy pep8 pyflakes 'ipython<4.1' sphinx numpydoc

@DougBurke
Copy link
Contributor Author

@olaurino - I'm looking at how to get a conda build of the docs (useful to test that the docs still build). If I have a fresh checkout of the feature/sphinx-build branch, I get:

% conda create -n=sherpa-sphinx python=2.7 matplotlib astropy pep8 pyflakes 'ipython<4.1' sphinx numpydoc
% source activate sherpa-sphinx
% python setup.py build_sphinx
running build_sphinx
Running Sphinx v1.4.1
loading pickled environment... not yet created
building [mo]: targets for 0 po files that are out of date
building [html]: targets for 17 source files that are out of date
updating environment: 17 added, 0 changed, 0 removed
reading sources... [ 11%] data/index                                            

>>>-------------------------------------------------------------------------
Exception in /home/djburke/sherpa/sherpa-sphinx/docs/data/index.rst at block ending on line 29
Specify :okexcept: as an option in the ipython:: block to suppress this message
---------------------------------------------------------------------------
ImportError                               Traceback (most recent call last)
<ipython-input-3-aaba73a7fef6> in <module>()
----> 1 from sherpa import data

/home/djburke/sherpa/sherpa-sphinx/sherpa/data.py in <module>()
     27 from itertools import izip
     28 import numpy
---> 29 from sherpa.utils.err import DataErr, NotImplementedErr
     30 from sherpa.utils import SherpaFloat, NoNewAttributesAfterInit, \
     31      print_fields, create_expr, calc_total_error, bool_cast, \

/home/djburke/sherpa/sherpa-sphinx/sherpa/utils/__init__.py in <module>()
     38 # Note: _utils.gsl_fcmp is not exported from this module; is this intentional?
     39 from unittest import skipIf
---> 40 from sherpa.utils._utils import calc_ftest, calc_mlr, igamc, igam, \
     41     incbet, gamma, lgam, erf, ndtri, sao_fcmp, rebin, \
     42     hist1d, hist2d, sum_intervals, neville, sao_arange

ImportError: No module named _utils
<<<-------------------------------------------------------------------------
...
... lots more
... 

We'll need to look into this.

@dtnguyen2
Copy link
Contributor

I haven't looked at the code yet, but I think we only export sao_fcmp which
is built on top of gsl_fcmp because it gsl_fcmp had a few bugs

On Sun, May 1, 2016 at 10:58 AM, Doug Burke notifications@github.com
wrote:

@olaurino https://github.com/olaurino - I'm looking at how to get a
conda build of the docs (useful to test that the docs still build). If I
have a fresh checkout of the feature/sphinx-build branch, I get:

% conda create -n=sherpa-sphinx python=2.7 matplotlib astropy pep8 pyflakes 'ipython<4.1' sphinx numpydoc
% source activate sherpa-sphinx
% python setup.py build_sphinx
running build_sphinx
Running Sphinx v1.4.1
loading pickled environment... not yet created
building [mo]: targets for 0 po files that are out of date
building [html]: targets for 17 source files that are out of date
updating environment: 17 added, 0 changed, 0 removed
reading sources... [ 11%] data/index


Exception in /home/djburke/sherpa/sherpa-sphinx/docs/data/index.rst at block ending on line 29

Specify :okexcept: as an option in the ipython:: block to suppress this message

ImportError Traceback (most recent call last)
in ()
----> 1 from sherpa import data

/home/djburke/sherpa/sherpa-sphinx/sherpa/data.py in ()
27 from itertools import izip
28 import numpy
---> 29 from sherpa.utils.err import DataErr, NotImplementedErr
30 from sherpa.utils import SherpaFloat, NoNewAttributesAfterInit,
31 print_fields, create_expr, calc_total_error, bool_cast, \

/home/djburke/sherpa/sherpa-sphinx/sherpa/utils/init.py in ()
38 # Note: _utils.gsl_fcmp is not exported from this module; is this intentional?
39 from unittest import skipIf
---> 40 from sherpa.utils._utils import calc_ftest, calc_mlr, igamc, igam,
41 incbet, gamma, lgam, erf, ndtri, sao_fcmp, rebin,
42 hist1d, hist2d, sum_intervals, neville, sao_arange

ImportError: No module named _utils
<<<-------------------------------------------------------------------------
...
... lots more
...

We'll need to look into this.


You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub
#197 (comment)

@DougBurke
Copy link
Contributor Author

@dtnguyen2 - that comment was something I put in a while ago when doing doc work. It is something to think about (or at least add a note somewhere to say whether it should be exported or not), but it's not the cause of this build failure. I think the build failure is related to the issues we've seen where it's possible to get confused if you use both the test and install targets.

@dtnguyen2
Copy link
Contributor

Back in the day, everyone was encouraged to use sao_fcmp instead of
gsl_fcmp but it has been a while since that manifesto was declared and I
suppose I could do a search through the code but I don't have my computer
handy. I suspect though that because we have not added any C/C++ code in a
while then it is probably free of gsl_fcmp

BTW, this is one of the sticky points about making our code BSD cause there
are ways to write fuzzy comparison for equality but not cmp (returning -1,
1 or 0)

On Sunday, May 1, 2016, Doug Burke notifications@github.com wrote:

@dtnguyen2 https://github.com/dtnguyen2 - that comment was something I
put in a while ago when doing doc work. It is something to think about (or
at least add a note somewhere to say whether it should be exported or not),
but it's not the cause of this build failure. I think the build failure is
related to the issues we've seen where it's possible to get confused if you
use both the test and install targets.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#197 (comment)

@DougBurke DougBurke self-assigned this May 2, 2016
@DougBurke
Copy link
Contributor Author

From a much-earlier discussion about whether it is possible to just upload documentation - perhaps created by travis builds - to readthedocs: I believe that it used to be possible, then was removed, then was on the wish list. There was even a bug report about it - see the comment to http://stackoverflow.com/questions/27228115/is-it-possible-to-upload-sphinx-pre-generated-html-documents-into-readthedocs-si - which I am sure was readable last week but is now a 404 error - namely readthedocs/readthedocs.org#1083

So it appears that this is currently not an option.

@olaurino
Copy link
Member

olaurino commented May 2, 2016

I'm looking at how to get a conda build of the docs (useful to test that the docs still build). If I have a fresh checkout of the feature/sphinx-build branch, I get:

@DougBurke I probably need more information to understand what you are trying to do and what is different from what you usually do. However, from the error message and the sequence of commands it looks like you are not building the python extensions "inplace". You may need to call sherpa_config (and possibly xspec_config) which will also run build_ext. Try looking at the very first section of setup.cfg, where some setuptools aliases are defined.

@DougBurke
Copy link
Contributor Author

@olaurino I was looking to see how to fit the build into the travis setup, which has a single python setup call. So I guess this means adding an alias so setup.cfg, as you mentioned (either to augment the default build_sphinx target or just add a new one). Probably worth talking about this particular issue face to face some time.

@olaurino
Copy link
Member

olaurino commented May 2, 2016

I thought you meant conda :D

Anyhow, we can indeed create a travis build for the documentation. Even better it would be to have the docs uploaded to be published, but I understand that is not possible with ReadTheDocs. I wonder if we could do it with GitHub pages, although the main issue with a non-RTD solution is that we would need to manage different branches/versions ourselves.

@DougBurke
Copy link
Contributor Author

@olaurino - I was able to get python setup.py build_sphinx to work. Yeah.

Unfortunately it takes too long for Travis - see https://travis-ci.org/sherpa/sherpa/jobs/127364198 - so I've commented it out of .travis.yml for now.

@olaurino
Copy link
Member

olaurino commented May 3, 2016

Could you set sphinx to be verbose and output to stdout?

@DougBurke
Copy link
Contributor Author

When I run sphinx on the command line it's plenty verbose, so I do not know what is going on with the Travis version (and the log is empty at this part, so not very helpful).

@olaurino
Copy link
Member

olaurino commented May 5, 2016

Maybe it is waiting for user's input?

@DougBurke
Copy link
Contributor Author

DougBurke commented May 5, 2016

https://travis-ci.org/sherpa/sherpa/jobs/127364198#L485 suggests you are probably correct, and that it's actually hanging at the conda installation stage. Oops.

Edited to add: let's see how https://travis-ci.org/sherpa/sherpa/jobs/128039153 does

Further edited to add: it looks to have worked - you can see the sphinx output starting at https://travis-ci.org/sherpa/sherpa/jobs/128039153 and the screen output looks as expected.

@olaurino
Copy link
Member

olaurino commented May 5, 2016

You mentioned that part of the documentation is built by running commands and saving the output, including images. Is that correct? So if we make an update to the code and all the tests but we forget to update the documentation we will catch that issue, right?

@DougBurke
Copy link
Contributor Author

If it causes an error then it can be automatically flagged (e.g. to fail the build). It is not perfect, as there are many cases that I could see that a commit would cause unwanted changes in behaviour, but not an actual error.

I don't think this is quite what you were thinking of, but https://travis-ci.org/sherpa/sherpa/jobs/128039153#L4091 shows a documentation warning which I believe can be turned into an error (it was in fact something I'm just about to investigate).

@olaurino
Copy link
Member

olaurino commented May 5, 2016

So, in other terms you get an error if the output cannot be generated, but there is no mechanism for checking the output is consistent. Am I on the right track? I wonder if there is a way to run something similar to doctests in sphinx, so that we can make sure the output does not change in unexpected ways, and if it does in expected ways that we update the documentation accordingly as part of the changeset. I would not consider this a priority but something valuable to add later on, if at all possible.

@taldcroft
Copy link
Contributor

I noticed that pandas uses the IPython directive for embedded code in their docs. I was wondering about doctesting as well. Astropy builds fail if there are any warnings coming from sphinx (which was a huge pain to get working the first time around). Astropy uses the Sphinx doctest framework with some custom extensions to handle floating point comparison and other nits that come up. The IPython directive looks like it can do doctesting, but I'm not sure how sophisticated it is. Astropy also has a nifty little widget to remove the >>> stuff and all outputs in the doc examples so one can copy/paste into an editor.

Stylistically I am not a huge fan of code examples being in IPython format rather than the classic format. All the whitespace makes examples take up too much vertical space and the In[2] just seem a distraction. Pandas tends to have very short examples per block, but in Sherpa there are many more instances of code blocks with 10 or more lines. But that's just my opinion from the sidelines (and I appreciate the convenience of @savefig). Overall this is amazing progress.

@taldcroft
Copy link
Contributor

@DougBurke - sorry, I haven't paid too much attention to the astropy build / setup infrastructure. I do see that in our travis setup we have this doctest build:

        # Check for sphinx doc build warnings - we do this first because it
        # runs for a long time. WCSAxes is a dependency for the docs.
        - os: linux
          env: PYTHON_VERSION=2.7 SETUP_CMD='build_sphinx -w'
               CONDA_DEPENDENCIES=$CONDA_ALL_DEPENDENCIES
               PIP_DEPENDENCIES='wcsaxes --no-deps'

The normal command line testing for astropy defaults to testing both docstrings and sphinx docs. This is fairly crucial for making sure docs correspond to the current code. (Sorry, I'm mixing two issues here, one is building the sphinx docs and checking for warnings which requires running sphinx, and the other is testing code examples in the code docstrings and sphinx RST docs, which does not require running sphinx).

If you enable doc testing then you pretty much need some mechanism for skipping individual lines (for things that cannot / should not be tested) and doing float compares. # doctest: +SKIP is the sphinx built-in, and astropy provides # doctest: +FLT_COMPARE (or something like that).

@DougBurke
Copy link
Contributor Author

For @taldcroft - I do agree that the current IPython sessions are visual train wrecks - in that the default IPython prompt takes up too much space. I haven't spent time on trying to optimise this; the IPython directive has some configuration variables but from the comments they don't seem to be quite what I would want and I haven't had the energy to investigate this any further.

@taldcroft
Copy link
Contributor

I looked at the IPython directive source code for a few minutes this morning. It would be easy enough to change the input and output prompts:

ipython_promptin:
    The string to represent the IPython input prompt in the generated ReST.
    The default is 'In [%d]:'. This expects that the line numbers are used
    in the prompt.
ipython_promptout:
    The string to represent the IPython prompt in the generated ReST. The
    default is 'Out [%d]:'. This expects that the line numbers are used
    in the prompt.

So e.g. ipython_promptin = '>>> ' and ipython_promptout = '' in conf.py might get most of the way there. But you still need to get rid of the extra vertical whitespace, and I don't immediately see a really clean way to do that. The end part of the process is here, and in theory you could hijack run() and have new code to take out blank lines. A bit ugly / hacky.

@DougBurke
Copy link
Contributor Author

@taldcroft - I wasn't sure, from the docs, whether this would work. I've just come across the following trying it out - with IPython 4.0.3 (so maybe changed in a newer version but savefig appears to be broken in them):

IPython/sphinxext/ipython_directive.py", line 655, in process_block
    input_prompt = self.promptin % lineno
TypeError: not all arguments converted during string formatting
Error in atexit._run_exitfuncs:

This is with

ipython_promptin = '>>> '
ipython_promptout = ''

I think it's the This expects that the line numbers are used in the prompt. comment.

@DougBurke
Copy link
Contributor Author

Or, perhaps, I need to change the 'IPython [1]: '.... parts to use '>>> ' in the documentation as well as the setting in conf.py. That'll be a larger change...

@taldcroft
Copy link
Contributor

Ack, looks like the ipython_prompt* vars must have a format conversion token in them. So never mind.

@DougBurke
Copy link
Contributor Author

Yup - that's my conclusion too.

I do have to subclass the ipython directive (to support grabbing the logging output), so it may be possible to override this behaviour, but I'm worried it won't be simple (I haven't looked at the code, in part out of worries over GPL vs BSD).

@taldcroft
Copy link
Contributor

I've looked at the code and it is relatively straightforward and it wouldn't be that hard to fix, it's just that the bits you need to fix are deep within long methods. So in theory you could copy that whole method into your subclass and fix it, but that is a bit fragile obviously. I think it would be safer to just copy the whole module. Again you can't guarantee that IPython does not make some change that breaks your local version, but since the breakage is basically limited to developers that's not a huge risk.

I don't think GPL vs BSD is an issue. My understanding is that BSD-licensed code is "GPL-compliant", i.e. you can always include BSD bits into a GPL project with no problem.

@taldcroft
Copy link
Contributor

Plan B might be an upstream patch to IPython to support classic mode (like ipython --classic) in their sphinx directive. Nice Friday afternoon patch. 😄

@DougBurke
Copy link
Contributor Author

@olaurino - you may be interested in https://github.com/astrofrog/pytest-mpl for testing matplotlib output (it requires a reference image to compare against).

@olaurino
Copy link
Member

olaurino commented May 6, 2016

@DougBurke awesome... too bad it pulls in nose as a dependency, but I guess I can live with that. Also, I don't think this is limited to matplotlib, I guess we could use it for chips tests as well, at least for new tests.

thanks!

@DougBurke
Copy link
Contributor Author

A current "sticking point" is astropy/astropy-helpers#236 which could well be a configuration issue, or user error (ie I've done something wrong).

@DougBurke
Copy link
Contributor Author

DougBurke commented Jun 5, 2016

@olaurino - I have included a Travis-CI documentation build in this branch, which has been going well until I tried to make it build the XSPEC module as well. My guess is that the following is due to the build_sherpa target not quite correct, but I need a fresh pair of eyes on it.

In a recent (failing) build of the feature/sphinx-docs branch, the _xspec.o code can't be build because -lwcs can't be found - e.g. https://travis-ci.org/sherpa/sherpa/jobs/135477054#L4113 - which only occurs for the build_sphinx option. In the same set of builds the XSPEC build is fine - e.g. see the equivalent build line works https://travis-ci.org/sherpa/sherpa/jobs/135477055#L4341

These are both for Travis build 990 - the failing build is Travis 990.2 and the successful build is Travis 990.3

The .travis.yml file for this build is

- env: XSPECVER="12.9.0i" FITS="astropy" INSTALL_TYPE=build_sphinx TEST=none MATPLOTLIBVER=1.5
and the build_sphinx config alias is set at
build_sphinx = sherpa_config xspec_config develop build_sphinx

I wonder if I should replace develop with something else in the following?

build_sphinx = sherpa_config xspec_config develop build_sphinx

@olaurino
Copy link
Member

olaurino commented Jun 7, 2016

I'll try and have a look today.

@DougBurke
Copy link
Contributor Author

I've rebased the branch (to match master) and moved it to my fork of Sherpa:

https://github.com/DougBurke/sherpa/tree/feature/sphinx-docs

I have left around the old branch on sherpa just to confuse people.

It's probably time for me to go through the branch and clean it up (essentially, create a new branch with just the bits I care about).

@cdeil
Copy link
Contributor

cdeil commented Sep 14, 2018

Do you have Sphinx docs now? Is a HTML version online already?

@DougBurke
Copy link
Contributor Author

It can be found at https://sherpa.readthedocs.io/en/latest/ [*] but I caution that the main aim was to get the process and overall "shape" of the documentation with this PR; the content needs more work. But please point out where you find it confusing/missing/whatever.

[*] I need to send a PR to update the README.md with this location rather than the test version. It also may change in the future.

@cdeil
Copy link
Contributor

cdeil commented Sep 15, 2018

This is awesome, thank you!

Yes, mentioning it in the README or whereever else people might look for a link to the docs would be very useful.

I don't have time to read much now, but I'll point this out on the Gammapy slack and encourage people to send pull requests or at least file issues with suggestions.

@cdeil
Copy link
Contributor

cdeil commented Sep 15, 2018

One issue I noticed is that it looks like the Sphinx search isn't working:
https://sherpa.readthedocs.io/en/latest/search.html?q=cash&check_keywords=yes&area=default

At least I use it all the time for Sphinx docs, so if you can get it to work that would be very useful. Note that RTD does stuff to your docs that often is very convenient and useful (such as the version selector JS applet), but sometimes it also screws something up, so if Sphinx search is working with a normal HTML build, it's probably some RTD issue or setting.

screen shot 2018-09-15 at 11 11 21

@cdeil
Copy link
Contributor

cdeil commented Sep 15, 2018

I see that the API docs are spread across multiple pages, e.g.

Maybe a section near the start of the docs that summarises and links to the API docs listings would be useful? Since the docs pages are so extensive and the navbar on the left doesn't mention sub-packages like sherpa.optmethods etc, at the moment it's not obvious how to find the API docs section for a given sub-package.

But that's a small comment, it's not clear if users would find the "Sherpa sub-packages / API overview section" even if you were to add it.

@DougBurke
Copy link
Contributor Author

Hmmm, the search works in the test version (which I believe should essentially be the same as the released version)

https://sherpa-test.readthedocs.io/en/latest/search.html?q=cash&check_keywords=yes&area=default#

One of the less-than-ideal parts of RTD is the lack of visibility/ease of debugging for issues such as this.

@cdeil if you or your team do have more doc-related items can you open a new issue so they don't get missed? ta

@DougBurke
Copy link
Contributor Author

For the "how to document the API" part: I'm not particularly happy with the current layout, but also couldn't come up with a better scheme.

@cdeil
Copy link
Contributor

cdeil commented Sep 15, 2018

if you or your team do have more doc-related items can you open a new issue so they don't get missed?

Will do from now on.

@olaurino and @DougBurke - Thank you for all your work to modernise and maintain Sherpa. It's much appreciated!

@DougBurke
Copy link
Contributor Author

@cdeil - I believe the search issues you discuss at #197 (comment) have now been fixed. I see there's been some fun things going on - e.g. sphinx-doc/sphinx#5460 - so I assume it's related to this.

@cdeil
Copy link
Contributor

cdeil commented Sep 28, 2018

Yes, looks like the Sphinx search issue is resolved. Thanks!

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

No branches or pull requests

5 participants