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

toctree content regression in sphinx 1.3b3 #1822

Closed
jschueller opened this issue Mar 31, 2015 · 11 comments · Fixed by #1892
Closed

toctree content regression in sphinx 1.3b3 #1822

jschueller opened this issue Mar 31, 2015 · 11 comments · Fixed by #1892

Comments

@jschueller
Copy link
Contributor

we're no longer able to build openturns sphinx documentation since 1.3b3:

Running Sphinx v1.4a0+
making output directory...
loading pickled environment... not yet created
[autosummary] generating ...
...
reading sources... [100%]
user_manual/user_manual
...                                                                                                                  
looking for now-outdated files... none found
pickling environment... done
checking consistency... 
Warning, treated as error:
/home/schueller/projects/openturns/schueller/build/python/src/sphinx_build/user_manual/_generated/openturns.ANCOVA.rst:: WARNING: document isn't included in any toctree

using git-bisect I found the regression to be at 21b8384 (for PR #1663):
could it be that this commit somehow stripped a class from toctree ?

@christianbrodbeck
Copy link

I think I might be having the same problem. Classes that are defined in package/_module.py (e.g. class package._module.MyClass, imported in package.__init__.py and listed in the doc like

.. currentmodule:: package
  :toctree: generated

   MyClass

just disappeared form the documentation. Commenting lines 273 and 274 in sphinx/ext/autosummary/__init__.py that were added in 21b8384 brought the classes back into my documentation.

@jschueller jschueller changed the title regression in sphinx 1.3b3 toctree content regression in sphinx 1.3b3 Apr 2, 2015
@jschueller
Copy link
Contributor Author

@lsaffre, maybe you could tell us more about this commit ? I hope removing the two lines added from sphinx/ext/autosummary/init.py in 21b8384 like @christianbrodbeck mentioned does not introduce more regressions towards #1061 and #1656.
maybe the comment of @embray in #1061 is useful

@lsaffre
Copy link
Contributor

lsaffre commented Apr 2, 2015

Looking from the source code (didn't test) I'd say that these two lines were for #1061 while the other changes were for #1656, and it was just an administrative mistake to solve two tickets with one changeset.

@jschueller
Copy link
Contributor Author

from what I understood this was a backward-incompatible change to strip imported modules.

@lsaffre
Copy link
Contributor

lsaffre commented Apr 2, 2015

Ha, I finally found my blog entry which describes the problem that was solved by these two lines:
http://luc.lino-framework.org/blog/2014/1219.html
(Summary: my code imports a class from a third-party package, and this class has a docstring which contains invalid reStructuredText markup. This broke my build since I want it to build without warnings. I want autosummary to document all my classes, but not those imported into my package from a third-party package)

@jschueller
Copy link
Contributor Author

ok, thanks for the explanation, unfortunately your changes broke several documentations. what about @shimizukawa suggestion to have a keyword imported, and the stack overflow patch mentioned in #1061 ?

@lsaffre
Copy link
Contributor

lsaffre commented Apr 2, 2015

Yes, that would be perfect! TIA for implementing it, and I'll do my best to help.

@shoyer
Copy link

shoyer commented Jun 1, 2015

This has broken my docs, and unfortunately, it's a pain to revert to earlier versions of sphinx on ReadTheDocs.

Is there a work around for this issue?

@jschueller
Copy link
Contributor Author

see pr #1828

@amueller
Copy link

amueller commented Jun 9, 2015

@jschueller
Copy link
Contributor Author

@lsaffre @shimizukawa @Eric89GXL maybe this is a better solution: #1892

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

Successfully merging a pull request may close this issue.

5 participants