readthedocs / sphinx-autoapi

A new approach to API documentation in Sphinx.
https://sphinx-autoapi.readthedocs.io/
MIT License
432 stars 128 forks source link

feat: allow objects to render in their own pages #399

Closed jorgepiloto closed 6 months ago

jorgepiloto commented 1 year ago

Fix #226 by allowing certain objects to be rendered in single pages. Main changes are:

jorgepiloto commented 1 year ago

Thanks for this amazing package @ericholscher @AWhetter.

I would like to request your guidance and feedback to complete this pull-request. I tried it to have the lowest impact as possible on the default templates.

For writing the tests, I created a copy of the tests/python/pypackagecomplex directory. This copy makes use of the new "render in a single page" feature for objects of class type.

These are the warnings raised by the tests:

/sphinx-autoapi/tests/python/pypackagecomplex_single_page/autoapi/complex/wildall/simple/SimpleClass.rst:7: WARNING: duplicate object description of SimpleClass, other instance in autoapi/complex/wildall/SimpleClass, use :no-index: for one of them
/sphinx-autoapi/tests/python/pypackagecomplex_single_page/autoapi/complex/wildall/simple/SimpleClass.rst:14: WARNING: duplicate object description of SimpleClass.simple_method, other instance in autoapi/complex/wildall/SimpleClass, use :no-index: for one of them
looking for now-outdated files... none found
pickling environment... done
checking consistency... /sphinx-autoapi/tests/python/pypackagecomplex_single_page/autoapi/complex/wildall/simple/NotAllClass.rst: WARNING: document isn't included in any toctree

Do you have any suggestions on how to fix those? As far as I understand, only one reference to an object should exist. However, I am not really sure where should I impose this condition.

jorgepiloto commented 11 months ago

Testing this with exceptions.

jorgepiloto commented 11 months ago

The failing tests is very strange. I can reproduce locally if I use tox. However, if I run the following command inside the tests/python/pypackageexample:

sphinx-build . _build/html -W

Everything builds as expected and the _build/html/autoapi/example/_private_module/index.html is present.

AWhetter commented 8 months ago

I don't seem to have permissions to push to this branch. I've made some progress on writing the tests on the readthedocs:feat/single-page-option branch. Feel free to use any additional commits from there, and I'll continue to check here for updates before I made any changes on my own branch.

jorgepiloto commented 8 months ago

Hi @AWhetter, thanks for keeping an eye on this. I saw the modifications you applied in the upstream branch. Is it fine if I merge your branch with this one and push your changes? This way you still get credit from your commits.

jorgepiloto commented 8 months ago

After merging your changes, @AWhetter, I noted that you removed the output_child_rst. I wrote this function to recursively generate children of children.

This allows to get the structure you propose in your tests. However, this issue still shows up:

Commit https://github.com/readthedocs/sphinx-autoapi/commit/d7b2c0876d099313745767a072461c013a8bdd1b generates the structure you suggested, @AWhetter. However, I am wondering which is the best approach to render classes declared within classes.

Main issue is that within the RST templates there are calls to the render() method. Thus, for the case of the Foo class that defines a Meta class inside of it, the render is weird.

jorgepiloto commented 8 months ago

The latest commits ensure the desired structure. I had to introduce a recursive function named output_child_rst.

Also, I fixed some minor issues in the module.rst so that it matches previous structure.

jorgepiloto commented 8 months ago

Still need to address the TODO in the class.rst for including a toctree linking to the children when page level extends beyond class.

jorgepiloto commented 8 months ago

The TODO in the class.rst has been addressed in https://github.com/readthedocs/sphinx-autoapi/pull/399/commits/3c32fd401062846977c9f0b30bcd4d84aa83a265.

This generates toctrees for all the required pages if needed. Also, when rendering nested classes, I avoided creating those in their own page. This ensures that the generated autoapi structure reflects the actual structure of the package.

Focusing now on completing the tests.

jorgepiloto commented 8 months ago

Finally! This is working really nice. It supports all the capabilities @AWhetter asked for. I am glad that we extended to these. Honestly, this looks super flexible!

jorgepiloto commented 8 months ago

The latest modifications by @AWhetter as way more elegant and readable than my recursive function.

After giving a try to this in some of our projects, I could find the following issues:

jorgepiloto commented 8 months ago

It looks like for class Class(Expression) a page named args.rst is being created.

jorgepiloto commented 8 months ago

Last commit fixes the issues with exceptions.

jorgepiloto commented 7 months ago

Commit https://github.com/readthedocs/sphinx-autoapi/pull/399/commits/2ef9a9d3f34090138a4ba427f3be17bfed1f87c1 ensures that if __all__ exists, only objects included in this attribute are rendered.

AWhetter commented 6 months ago

I'm closing this because it has been merged with further changes into main

jorgepiloto commented 6 months ago

Thanks for all the feedback and support, @AWhetter. Glad to see this is finally merged into main branch 🚀