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

Sphinx apidoc extension #4101

Closed

Conversation

stephenfin
Copy link
Contributor

@stephenfin stephenfin commented Oct 1, 2017

Subject: Make 'sphinx-apidoc' an extension

Feature or Bugfix

  • Feature

Purpose

  • This is an alternative approach to Exntend setuptools command to include sphinx-apidoc #1135. Rather than modifying the 'build_sphinx' setuptools extension, we allow 'sphinx-apidoc' to be run as an extension.

    This is very similar to what we do with 'sphinx-autogen', which can be run automatically using the autosummary_generate configuration option instead.

    Not all configuration options are exposed - I wanted some insight into which of these I should expose before doing so.

    My eventual goal would be to remove both the sphinx-apidoc and sphinx-autogen binaries at some point, but that's a Sphinx 2.0+ target. I will open an RFE issue to do this.

  • There are no specific environment requirements for this change. Tests are included.

Detail

  • Register a 'sphinx.ext.apidoc' extension and related configuration options.
  • Add tests and documentation for same

Relates

@tk0miya
Copy link
Member

tk0miya commented Oct 2, 2017

@shimizukawa could you check this please?
I feel this is very similar to autosummary. Is this helpful?

@stephenfin stephenfin force-pushed the sphinx-apidoc-extension branch 3 times, most recently from 39386c2 to b7a305a Compare March 22, 2018 11:22
@stmcginnis
Copy link

We have some cases where this capability would be useful. The output is different enough from autosummary that I don't think we would want to switch over to using that, but we would like to avoid the need to call sphinx-autodoc separately in our pipeline.

@stephenfin
Copy link
Contributor Author

stephenfin commented Mar 22, 2018

We have some cases where this capability would be useful. The output is different enough from autosummary that I don't think we would want to switch over to using that, but we would like to avoid the need to call sphinx-autodoc separately in our pipeline.

To expand on this, pbr currently expands the build_sphinx setuptools command to optionally call sphinx-apidoc before building docs. We've had issues maintaining this extension and would like to remove it from pbr. However, this necessitates having a replacement in place.

I feel this is very similar to autosummary. Is this helpful?

I took some time today to compare these. There are a number of differences I see here:

  • autosummary requires Python module paths (e.g. foo.bar), whereas sphinx-apidoc works on paths (src/my_package). The former means the package must be installed in the current (virtual) environment. The latter requires only the dependencies be installed.
  • autosummary is implemented as a directive combined with an optional event hook/callback to build stubs. sphinx-apidoc is currently implemented as an executable and this PR extends it to use an event hook/callback.
  • autosummary takes a "define what you want to include" approach to documentation. There is no clear way to include all but one file. sphinx-apidoc takes a "define what you want to exclude" approach, meaning you can document all except a given path.
  • autosummary's output is very different from the output of sphinx-apidoc. This is configurable, but not on a global basis (which means every project that wants its output in the sphinx-apidoc style would need to insert the same boilerplate directive.

Personally, I think these are very different and it's not really fair to compare them. We could extend autosummary to get what we want (and I've stated what I think would be necessary below) but I don't know if it makes sense to do so. I would like input here before continuing any further.

  1. Add support for file paths in the directive. I'm not sure how we could distinguish between the two of these. Maybe another flag or a prefix like this:

    .. autosummary::
        path:../oslo.config
    
  2. Add support for both a wildcard and exclude syntax. In my mind, this would look something like this:

    .. autosummary::
        oslo_config.*
        !oslo_config.tests
    
  3. Either convert the existing (ugly, IMO) autosummary templates to generate something akin to the output of sphinx-apidoc, or add the new templates plus another flag to switch to this style.

  4. Add some global configuration options that allow you to set defaults for the directive options. This is mainly needed so we can set those defaults from a theme to avoid people needing to copy-paste boilerplate directives.

If you ask me, this is going to be far more difficult to use and result in far more code than the design I have currently proposed. As noted above though, I'd love to get some input here.

@stephenfin
Copy link
Contributor Author

This is failing because of #4771. We need to resolve that before we can progress here.

This is an alternative approach to sphinx-doc#1135. Rather than modifying the
'build_sphinx' setuptools extension, we allow 'sphinx-apidoc' to be run
as an extension.

This is very similar to what we do with 'sphinx-autogen', which can be
run automatically using the 'autosummary_generate' configuration option
instead.

We may wish to remove the 'sphinx-apidoc' binary in the future, but
that's a decision for another day.

Signed-off-by: Stephen Finucane <stephen@that.guru>
@codecov
Copy link

codecov bot commented Mar 23, 2018

Codecov Report

Merging #4101 into master will increase coverage by 0.02%.
The diff coverage is 75.67%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4101      +/-   ##
==========================================
+ Coverage   82.13%   82.16%   +0.02%     
==========================================
  Files         283      289       +6     
  Lines       37671    38034     +363     
  Branches     5838     5898      +60     
==========================================
+ Hits        30942    31250     +308     
- Misses       5418     5463      +45     
- Partials     1311     1321      +10
Impacted Files Coverage Δ
tests/test_ext_apidoc.py 95.41% <100%> (+0.11%) ⬆️
tests/roots/test-ext-apidoc/apidoc_dummy_module.py 66.66% <66.66%> (ø)
sphinx/ext/apidoc.py 81.02% <72%> (+4.5%) ⬆️
sphinx/apidoc.py 0% <0%> (ø)
sphinx/quickstart.py 0% <0%> (ø)
sphinx/__init__.py 60% <0%> (ø)
sphinx/errors.py 70% <0%> (ø)
sphinx/search/__init__.py 96% <0%> (ø)
sphinx/ext/autodoc/__init__.py 85.27% <0%> (+0.13%) ⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 69383f8...782ef62. Read the comment docs.

stephenfin added a commit to sphinx-contrib/apidoc that referenced this pull request Mar 27, 2018
This is mostly based on the extension submitted as a pull request for
Sphinx [1].

[1] sphinx-doc/sphinx#4101

Signed-off-by: Stephen Finucane <stephen@that.guru>
@jakirkham
Copy link

FWIW lots of people try to do something similar. ( readthedocs/readthedocs.org#1139 ) Am sure they would also appreciate proper support in Sphinx for this use case. :)

@stephenfin
Copy link
Contributor Author

FWIW lots of people try to do something similar. ( readthedocs/readthedocs.org#1139 ) Am sure they would also appreciate proper support in Sphinx for this use case. :)

Agreed. Do note that the the version needs some rework but the idea is sound. I'm keeping this open until I can get some buy in from the maintainers (or at least told it's never going to happen). I essentially need to drag in the learning from https://github.com/sphinx-contrib/apidoc (the first version wasn't perfect, heh)

@stephenfin
Copy link
Contributor Author

I've spun this into a separate plugin so I'm going to drop this for now. We can pick it up again if a good reason to place this in core pops up.

@stephenfin stephenfin closed this Dec 21, 2019
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants