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

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

Closed
wants to merge 44 commits into from

Conversation

jorgepiloto
Copy link
Contributor

@jorgepiloto jorgepiloto commented Aug 21, 2023

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

  • New configuration variable autoapi_own_page_level
    This configuration variable accepts one of package, module, exception, class, function, method, property, attribute, data. Single pages are generated until the desired depth level. For example:

    Module depth

    • example_package/moduleA.rst

    Class depth

    • example_package/moduleA/index.rst
    • example_package/moduleA/ClassA.rst

    Function depth

    • example_package/moduleA/index.rst
    • example_package/moduleA/functionA.rst
    • example_package/moduleA/ClassA.rst

    Property depth

    • example_package/moduleA/index.rst
    • example_package/moduleA/functionA.rst
    • example_package/moduleA/ClassA/index.rst
    • example_package/moduleA/ClassA/propertyA.rst
  • Adapt default templates to operate with this new option
    Any template contains the two variables is_own_page and own_page_types. Using these two variables it is possible to customize the templates to operate with or without single pages. Default behavior will be the one prior to introducing this feature.

autoapi/mappers/base.py Outdated Show resolved Hide resolved
@jorgepiloto jorgepiloto changed the title feat: render objects in their own pages feat: allow objects to render in their own pages Aug 21, 2023
@jorgepiloto jorgepiloto marked this pull request as ready for review September 4, 2023 10:39
@jorgepiloto
Copy link
Contributor Author

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.

Copy link
Collaborator

@AWhetter AWhetter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you so much for your patience, and for making such a large contribution! 🎊

I've made some comments, and also pitched a couple of suggestions that it would be great to have your opinion on.

Please could we add some documentation for this change, as you've already noted in your description. Documentation makes it a bit easier to review because it means that I'll better understand how you intend the feature to work. Also it'll force you to explain how the change works, and if it's hard to explain then it's going to be hard for users to understand.

Finally, you'll need to include a news fragment through towncrier, to get the tests to pass. The README explains how to do this.

autoapi/templates/python/property.rst Outdated Show resolved Hide resolved
autoapi/templates/python/module.rst Outdated Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need to test absolutely everything that the pypackagecomplex test was created for (eg the rendering of binary data, different docstring styles, wildcard imports). So should we simplify this test case down and maybe come up with our own test case that checks for only what's relevant for single page rendering?

autoapi/mappers/base.py Outdated Show resolved Hide resolved
autoapi/mappers/base.py Outdated Show resolved Hide resolved
autoapi/extension.py Outdated Show resolved Hide resolved
@jorgepiloto
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Contributor Author

The TODO in the class.rst has been addressed in 3c32fd4.

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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:

  • If the tool encounters a class that inherits from a Exception, its fails.
  • The summary table at module level breaks for classes and no longer links despite showing the right names and summary docstrings.

@jorgepiloto
Copy link
Contributor Author

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

@jorgepiloto
Copy link
Contributor Author

Last commit fixes the issues with exceptions.

@jorgepiloto
Copy link
Contributor Author

Commit 2ef9a9d ensures that if __all__ exists, only objects included in this attribute are rendered.

@AWhetter
Copy link
Collaborator

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

@AWhetter AWhetter closed this Mar 29, 2024
@jorgepiloto
Copy link
Contributor Author

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

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

Successfully merging this pull request may close these issues.

Allow the choice of classes having their own page
3 participants