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

Closes #4334: sphinx-apidoc: References to non-existing files in TOC #4335

Merged
merged 1 commit into from
Jan 1, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ Features added
Bugs fixed
----------

* #4334: sphinx-apidoc: Don't generate references to non-existing files in TOC
* #4206: latex: reST label between paragraphs loses paragraph break
* #4231: html: Apply fixFirefoxAnchorBug only under Firefox
* #4221: napoleon depends on autodoc, but users need to load it manually
Expand Down
7 changes: 6 additions & 1 deletion sphinx/apidoc.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,12 @@ def create_package_file(root, master_package, subroot, py_files, opts, subs, is_
text += '\n'

# build a list of directories that are szvpackages (contain an INITPY file)
subs = [sub for sub in subs if path.isfile(path.join(root, sub, INITPY))]
# and also checks the INITPY file is not empty, or there are other python
Copy link
Contributor

Choose a reason for hiding this comment

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

an empty INITPY is still a valid INITPY making the directory a package as opposed to namespace

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True. But before this patch there would be a reference to that package, although the package .rst file would never be created. It seems to have been the behaviour of apidoc.py since many years to exclude those packages.

See:

sphinx/sphinx/apidoc.py

Lines 159 to 169 in 87cebf1

if INITPY in py_files:
# we are in package ...
if (# ... with subpackage(s)
subs
or
# ... with some module(s)
len(py_files) > 1
or
# ... with a not-to-be-skipped INITPY file
not shall_skip(path.join(root, INITPY))
):

That's why I'm reluctant to change an existing deliberate design, but tried to fix the inconsistency. If we change the behaviour in apidoc.py to not skip empty init.py files in shall_skip it'd still be consistent. @birkenfeld ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good.

# source files in that folder.
# (depending on settings - but shall_skip() takes care of that)
subs = [sub for sub in subs if not
shall_skip(path.join(root, sub, INITPY), opts)]

# if there are some package directories, add a TOC for theses subpackages
if subs:
text += format_heading(2, 'Subpackages')
Expand Down
Empty file.
16 changes: 16 additions & 0 deletions tests/roots/test-apidoc-toc/mypackage/main.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
#!/usr/bin/env python
import os

import mod_resource

import mod_something


if __name__ == "__main__":
print("Hello, world! -> something returns: {}".format(mod_something.something()))

res_path = \
os.path.join(os.path.dirname(mod_resource.__file__), 'resource.txt')
with open(res_path) as f:
text = f.read()
print("From mod_resource:resource.txt -> {}".format(text))
1 change: 1 addition & 0 deletions tests/roots/test-apidoc-toc/mypackage/no_init/foo.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
MESSAGE="There's no __init__.py in this folder, hence we should be left out"
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
This is a text resource to be included in this otherwise empty module. No python contents here.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
"Subpackage Something"
77 changes: 77 additions & 0 deletions tests/test_apidoc.py
Original file line number Diff line number Diff line change
Expand Up @@ -188,3 +188,80 @@ def test_extension_parsed(make_app, apidoc):
with open(outdir / 'conf.py') as f:
rst = f.read()
assert "sphinx.ext.mathjax" in rst


@pytest.mark.apidoc(
coderoot='test-apidoc-toc',
options=["--implicit-namespaces"],
)
def test_toc_all_references_should_exist_pep420_enabled(make_app, apidoc):
"""All references in toc should exist. This test doesn't say if
directories with empty __init__.py and and nothing else should be
skipped, just ensures consistency between what's referenced in the toc
and what is created. This is the variant with pep420 enabled.
"""
outdir = apidoc.outdir
assert (outdir / 'conf.py').isfile()

toc = extract_toc(outdir / 'mypackage.rst')

refs = [l.strip() for l in toc.splitlines() if l.strip()]
found_refs = []
missing_files = []
for ref in refs:
if ref and ref[0] in (':', '#'):
continue
found_refs.append(ref)
filename = "{}.rst".format(ref)
if not (outdir / filename).isfile():
missing_files.append(filename)

assert len(missing_files) == 0, \
'File(s) referenced in TOC not found: {}\n' \
'TOC:\n{}'.format(", ".join(missing_files), toc)


@pytest.mark.apidoc(
coderoot='test-apidoc-toc',
)
def test_toc_all_references_should_exist_pep420_disabled(make_app, apidoc):
"""All references in toc should exist. This test doesn't say if
directories with empty __init__.py and and nothing else should be
skipped, just ensures consistency between what's referenced in the toc
and what is created. This is the variant with pep420 disabled.
"""
outdir = apidoc.outdir
assert (outdir / 'conf.py').isfile()

toc = extract_toc(outdir / 'mypackage.rst')

refs = [l.strip() for l in toc.splitlines() if l.strip()]
found_refs = []
missing_files = []
for ref in refs:
if ref and ref[0] in (':', '#'):
continue
filename = "{}.rst".format(ref)
found_refs.append(ref)
if not (outdir / filename).isfile():
missing_files.append(filename)

assert len(missing_files) == 0, \
'File(s) referenced in TOC not found: {}\n' \
'TOC:\n{}'.format(", ".join(missing_files), toc)


def extract_toc(path):
"""Helper: Extract toc section from package rst file"""
with open(path) as f:
rst = f.read()

# Read out the part containing the toctree
toctree_start = "\n.. toctree::\n"
toctree_end = "\nSubmodules"

start_idx = rst.index(toctree_start)
end_idx = rst.index(toctree_end, start_idx)
toctree = rst[start_idx + len(toctree_start):end_idx]

return toctree