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

Allow using sphinx.ext.apidoc as an extension #12471

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

chrisjsewell
Copy link
Member

@chrisjsewell chrisjsewell commented Jun 24, 2024

A common use case,
is that users want to simply point sphinx towards a Python module, and have it generate documentation automatically.
(see e.g. readthedocs/readthedocs.org#1139)

This is not possible currently,
without a "pre-build" step of running the sphinx-apidoc CLI.
(and in fact there are numerous examples of users calling sphinx-apidoc within their conf.py to work around this)

This PR adds sphinx.ext.apidoc as a sphinx extension, to incorporate the source file generation into the sphinx build in a "formal" manner, using the API rather than the CLI.

related to #6829 (but does not close it)


This PR is currently in draft, since there are a number of TODOs in the code, and documentation needs to be added.

@chrisjsewell chrisjsewell marked this pull request as draft June 24, 2024 12:41
@chrisjsewell chrisjsewell requested a review from picnixz June 24, 2024 12:42
@chrisjsewell
Copy link
Member Author

@picnixz note this is still a draft, but perhaps you would like to provide some initial feedback

A common usecase,
is that users simply want to point sphinx towards a Python module,
and have it generate documentation automatically.

This is not possible currently,
without a "pre-build" step of running the `sphinx-autogen` CLI.

This PR adds `sphinx.ext.apidoc` as a sphinx extension,
to incorporate the source file generation into the sphinx build.
@AA-Turner AA-Turner changed the title ✨ Add sphinx.ext.apidoc extension Allow using sphinx.ext.apidoc as an extension Jul 3, 2024
Copy link
Member

@AA-Turner AA-Turner left a comment

Choose a reason for hiding this comment

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

Very brief skim:

@@ -0,0 +1,10 @@
"""So program can be started with ``python -m sphinx.apidoc ...``"""
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"""So program can be started with ``python -m sphinx.apidoc ...``"""
"""Command-line interface for the apidoc extension."""

from sphinx.locale import __
from sphinx.util.console import bold

from . import WARNING_TYPE, _remove_old_files, create_modules_toc_file, logger, recurse_tree
Copy link
Member

Choose a reason for hiding this comment

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

Avoid relative imports:

Suggested change
from . import WARNING_TYPE, _remove_old_files, create_modules_toc_file, logger, recurse_tree
from sphinx.ext.apidoc import WARNING_TYPE, _remove_old_files, create_modules_toc_file, logger, recurse_tree

Comment on lines 638 to +639
def main(argv: Sequence[str] = (), /) -> int:
"""Parse and check the command line arguments."""
"""Run the apidoc CLI."""
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps move main() into __main__.py?


# destination path should be relative to the source directory
# TODO account for Windows path?
if not (destination := _check_string(i, options, 'destination', True)):
Copy link
Member

Choose a reason for hiding this comment

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

Lone Booleans at call sites are often hard to interpret -- perhaps make required a keyword-only argument?

return options[key]


@dataclass
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps?:

Suggested change
@dataclass
@dataclass(frozen=True)

I'd also maybe set slots=True, but that is only Python 3.10+

Copy link
Member

Choose a reason for hiding this comment

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

According to our release plan, we should have dropped 3.9 some months ago so maybe we could first move to 8.x by dropping 3.9 (there are quite a lot of annoying things in between 3.9 and 3.10).

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

Successfully merging this pull request may close these issues.

4 participants