add sphinx documentation (fix #197) #302
Conversation
DougBurke
force-pushed
the
add-sphinx-docs
branch
from
604c495
to
bc63af5
Compare
|
Rebased. Tip was 604c495. |
|
I've updated my local web site with the latest version - e.g. http://hea-www.harvard.edu/~dburke/playground/sherpa/models/index.html#available-models to see the model documentation, including the XSPEC models. EDIT the version at https://sherpa-test.readthedocs.io/en/latest/ is likely to be more up-to date than my version. |
|
Rebased. Original tip was 0040d2f |
DougBurke
force-pushed
the
add-sphinx-docs
branch
from
0040d2f
to
2b4cd9b
Compare
DougBurke
force-pushed
the
add-sphinx-docs
branch
from
2b4cd9b
to
2291bc1
Compare
|
Rebased. Original tip was 2b4cd9b |
|
Have you tried to run doctests on the sphinx documentation? I would expect it to fail at this point, because docstrings need special massaging in order to run with doctest, but it would be interesting to know how badly they fail. |
|
I haven't tried doctest on this branch. I did play around with it in a previous attempt (hopefully it made it into the https://github.com/sherpa/sherpa/tree/feature/sphinx-docs branch but I don't have time to dig around there). At the moment I am not planning on investigating doctest, both from a technical side (there's enough issues I'm trying to get working already), and from the documentation side (not all code snippets make good or useful tests). |
DougBurke
changed the title
|
Rebased. Tip was 0cb1d92. Hopefully the |
DougBurke
force-pushed
the
add-sphinx-docs
branch
from
0cb1d92
to
72608ea
Compare
|
I'm not 100% sold on the travis build set up, but it passes for now! |
DougBurke
force-pushed
the
add-sphinx-docs
branch
from
24b2722
to
78bb6e5
Compare
|
Rebased but I forgot to check what the tip was before rebasing. Oops. |
|
Rebasing. Tip was a16a3f6 |
DougBurke
force-pushed
the
add-sphinx-docs
branch
from
a16a3f6
to
009a483
Compare
|
Rebasing. Tip was 009a4a3 |
DougBurke
force-pushed
the
add-sphinx-docs
branch
from
009a483
to
66baacd
Compare
DougBurke
changed the title
DougBurke
force-pushed
the
add-sphinx-docs
branch
from
2c5ed18
to
63d011a
Compare
|
About to rebase to address test failure due to upstream commits. Tip was 63d011a |
DougBurke
force-pushed
the
add-sphinx-docs
branch
from
63d011a
to
f410e08
Compare
|
About to rebase to squash all the sphinx-related commits (and separate out a recent docstring change). Original tip was 960a03a |
DougBurke
force-pushed
the
add-sphinx-docs
branch
from
960a03a
to
dbc2f18
Compare
|
My main concert with this PR is that the code in the sphinx documentation cannot be tested. I think this is really a bad idea, as the benefits of testing the docs automatically greatly outweighs the complexity required to configure the tests. I guess I lost that battle, and that even if I didn't the argument would now be "it takes too long now", which is exactly why I advocating for enabling automated tests when we started talking about this. I'll merge the PR as is and I'll skip crossing my fingers as that would be futile. |
|
We do not test any of the example code in the existing docstrings either. Shall we pull the whole code base until we fix this? |
|
No, that's not what I am saying. That's "legacy" docstrings, while the sphinx documentation is new and we could have enabled testing when we started if we wanted to. |
The Region and region_mask symbols are only exported from sherpa.astro.utils when the sherpa.astro.utils._region module is built, but this case was not being considered by sherpa.astro.data. Since the symbols could be used when un-pickling a state file, create a warning about missing support rather than raising an ImportErr (when it makes sense). There are no tests for this, and it's not clear if we can still build Sherpa without a _region module, so an alternative would be to just make it a required part of the Sherpa installation and remove this conditional code.
This makes the documentation easier to write, and easier for other code to parse (e.g. for documentation). It also allows for more-accurate function signatures.
Add docstrings. Explicit imports and style fixes (mainly adding/removing blank lines/characters).
Add an explicit calc_staterror function in the classes rather than setting it to the symbol exported by _statfcts. This is to make sure that the docstring of the method is picked up by the sub-classes (for the Sphinx documentation but should also help users).
It is still unclear whether this module is "safe" for users to import and use directly.
There is no functional change in this commit.
Remove unexported symbols, clean up a confusing code construct to be more Pythonic, and note an unused symbol (if_) in the printf definition.
The testing routines are now accessed directly from sherpa.utils.testing. This simplifies the utils module slightly, and is useful for building the documentation.
This is to support building the documentation via Sphinx without the need to build Sherpa. It is also easier for writers to check that the documentation is readable and meaningful, as well as providing a more- accurate signature for users. Improve module documentation.
The docstrings are added to sherpa/utils so that they can be picked up by Sphinx when _psf has not been built, to make them easier to edit, and improve the user experience by having accurate signatures. This is a first-go around on the documentation as it is not clear I have understood everything about these routines.
Having a reference in the summary line is not ideal, so move to the main text.
It appears that the LDFLAGS set up by the conda environment when it installs gfortran_linux-64 breaks the Fortran build because NumPy's distutils code takes the LDFLAGS setting wholesale, rather than appending them to its "current" settings. See numpy/numpy#1171 The LDFLAGS setting could be removed before calling the setup code, but it may be useful elsewhere in the code, so hardcode the addition of "-shared" to the LDFLAGS environment variable, but only if set to a non-empty value. This feels hacky; let's see how it works out. The suggestion is based on feedback from Thomas Holder to a question posed to the anaconda list - see https://groups.google.com/a/continuum.io/forum/#!topic/anaconda/Xw57CjIcBIU but all mistakes and confusion should be directed at the author of this commit.
The aim is to support builing the documentation without having to build Sherpa, as that appears to be the easiest way to get the documentation built on Read The Docs. The documentation build is Python 3.5 (or later) only: it is unclear whether this is due to the complicated import structure in Sherpa or changes to how the mock object works in Python 2.7 compared to 3.5. The auto-summary templates are taken from the AstroPy helpers project. Note that the documentation for the optimisation section is a rework of the material from the CIAO 3.4 version of the Sherpa manual, in particular the memo from Mark Birkinshaw, which can be seen at http://asc.harvard.edu/sherpa3.4/documents/manuals/html/refmethods.html Unfortunately the optimisers have changed since this memo: a) there's a lot fewer b) the scatter-shot versions have been completely rewritten so it's not clear if the advice section is still sensible. When building with Sphinx, there are a large number of warnings like the following: /home/djburke/sherpa/sherpa-add-sphinx-docs/sherpa/ui/utils.py:docstring of sherpa.ui.utils.Session.confidence:163: WARNING: Duplicate explicit target name: "1". Is this because the same docstring is being re-used for multiple functions (due to the inclusion of *.ui and Session object), an issue with the documentation format (I have looked but don't see it), some other issue, or a problem with Sphinx? It does mean that we can not run a "clean" build of the documentaition in Travis. One choice made in this documentation is not to auto-generate the output (since the aim is to be able to build the documentation without having to build Sherpa). There are scripts provided to generate the screen output (and PNG files), but this is not complete. Update the fit/simple-fit thread documentation This removes reference to a bug which has been fixed in Sherpa 4.10.0 (the need for hline in a resid plot), as well as fixing the documentation to address some errors. Added scripts to replicate the documents.
This comments out the recent change which was to support easy building of Sherpa using the conda-supplied gfortran. This was added to help out the Read The Docs build but that approach has since been abanded (i.e. the need to build Sherpa on Read The Docs). The change to pass through the F77 link arguments is controversial, since @olaurino feels it is error prone. It is also not clear if it leads to breaking the OS-X Travis CI build. This commit comments out the application of the extra arguments to the code that builds the extensions to see if this does cause the OS-X problem.
|
Rebasing to resolve conflicts, was fec0844. I'll wait for this to check out with Travis before merging anything else. |
olaurino
force-pushed
the
add-sphinx-docs
branch
from
dbc2f18
to
b85ac8a
Compare
This PR is ready for review, although I am likely to try and improve the documentation.
Summary
Create the Sherpa documentation using Sphinx to build, Travis-CI to test, and Read The Docs to build and display. The aim for the documentation is to build a sensible skeleton, but not to have a feature-complete set of documentation.
Notes
There are three parts to getting sphinx documentation
module docstrings)
Status
The outline of the documentation is present, as are much of the concepts, but there are missing areas and areas that need work.
All three points above have been addressed in this PR. I have set up a test build on Read The Docs - https://sherpa-test.readthedocs.io/en/latest/ - which requires manual re-building when new commits are made (so may not reflect the latest changes). Once the PR is integrated into the Sherpa branch I have a Read The Docs account set up to build the documentation from the master branch on every commit.
It is possible to use this PR but build the documentation in some other manner (e.g. via GitLab) as there are only a few small, text-only, configuration files specific to the Read The Docs environment:
readthedocs.ymldocs/environment.ymldocs/rtd-pip-requirementsIn order to get the documentation to build on Read The Docs, which does not support "install any package you need", the build is done without requiring a functional Sherpa installation. It does this by mocking required packages and the compiled modules within Sherpa. The only required Python packages for the build (other than Sphinx and
sphinx_rtd_theme) arenumpy(sincesetup.pyrequires it), andsix(although this probably also be mocked).Previous issues with evidence of mocked symbols - e.g. in the routines provided in
sherpa.astro.xspec, as described in https://sherpa-test.readthedocs.io/en/latest/model_classes/xspec_model.html - has been fixed.The output from the example code is not auto-generated: this is different to my previous approach in https://github.com/DougBurke/sherpa/tree/feature/sphinx-docs which was nice, but required the ability to build Sherpa and there was some interesting interaction between the Sherpa support code and some of the Sphinx utility routines with screen output.
Notes
All the commits up to "
Add a Sphinx build" are clean up, improvements, and moving information around in the existing code. It is not clear if all these changes are needed (since dropping support for building the documentation for Python 2.7 I have had less problems, but I have not gone tried the documentation build without these changes), but I believe they are worthwhile improvements anyway. They could also be cherry-picked into one or more PRs that are accepted before this one (getting these in sooner rather than later will reduce the amount of merge conflicts in our lives).Read The Docs issues
The main issue at the moment is that the build can time out, which has meant that I have removed the PDF and ePub generation to try and save time (they are also not useful at this moment). The runtime does seem somewhat random - presumably down to download times.
I was close to getting Read The Docs to be able to build Sherpa - which would have avoided the need to mock most things (although it may still be useful to mock out optional elements to reduce the round-trip time to get new documentation) by taking advantage of the conda support. Unfortunately there was a problem with the FORTRAN compiler (the conda environment didn't seem to be getting passed in), which scuppered these plans. There is a GitHub issue on the RTD page about this but I've not had any response.