Subpackage not in toc

[codingjoe]

Subject: Subpackage not in toc

Problem

In this commit it seems, there was a bug introduce. The bug does is the reverse of what the commit is trying to achieve.
Subpackages in version Sphinx (sphinx-apidoc) 1.6.6 are only picked up if they DO NOT contain a __init__.py file. Which doesn't make them packages but folder.

Procedure to reproduce the problem

mkdir -p parent/child
touch parent/__init__.py
touch parent/child/__init__.py

pip install sphinx==1.6.5
sphinx-apidoc -f --separate -o . .
cat parent.rst | grep Subpackages ; echo $?

pip install sphinx==1.6.6
sphinx-apidoc -f --separate -o . .
cat parent.rst | grep Subpackages ; echo $?

Environment info

• OS: macOS & Ubuntu 16.04
• Python version: 3.6.4
• Sphinx version: 1.6.6

[tk0miya]

Is this related with #4449? If true, it was already fixed at HEAD of stable branch.
Could you try it please?

[tk0miya]

Ah, sorry. it is not related with this.

FROM tk0miya/sphinx-html

RUN mkdir -p parent/child
RUN touch parent/__init__.py
RUN touch parent/child/__init__.py

RUN pip3 install sphinx==1.6.5
RUN sphinx-apidoc -f --separate -o . .
RUN cat parent.rst | grep Subpackages ; echo $?

[tk0miya]

With #4334, apidoc certainly ignores parent.child module if it doesn't have any elements under its namespace.
Please let me know your opinion.
What document should we generate for parent.child?

Note: #4449 fixed the problem if parent/child/__init__.py is empty but there are more modules under parent.child namespace.

[codingjoe]

@tk0miya this is still a major bug and effects all packages that implement django commands, as they are in mangement/commands/.
I still consider this wrong behavior, because a package that has a subpackage is not empty, because it has a package in it.
I think you should revert the commit. I don't see why someone would want to omit empty packages anyways. This could be an opt-in/out but no default.

[tk0miya]

The reverting causes broken toctree again (see #4334). So only my concern is what document should be generated for parent.child. empty page? only a title?
We need to discuss about it.

[codingjoe]

I would simply not omit empty packages. I would add them. So yes, empty file with a title.
If you don't want a file for your empty package, remove the package, right?

[tk0miya]

I'd not made a empty package ever, so I don't have strong opinion for this.
I feel both skipping the package and creating empty page are natural. So either behavior is fine to me.

@zufallsgenerator what do you think?

[zufallsgenerator]

I'd say in that case, keep it as it is.

…

On 16 Feb 2018 12:43, "Takeshi KOMIYA" ***@***.***> wrote: I'd not made a empty package ever, so I don't have strong opinion for this. I feel both skipping the package and creating empty page are natural. So either behavior is fine to me. @zufallsgenerator <https://github.com/zufallsgenerator> what do you think? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#4520 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AC-TQOvN0KqCfE2lvC2GZgkyZw3tkCpGks5tVWnLgaJpZM4Rw0GL> .

[codingjoe]

I guess you mean we should keep the behavior. Ok, that means we omit empty packages but do not consider a package empty if it does have a sub-package underneath.

[zufallsgenerator]

Sorry, I answered on the go (through an e-mail) and didn't get read the entire context. I now understand @codingjoe 's concern about the django commands that are in subfolders.

I think the entire reason why this is so hard to reason about, is the conditions that says that files that contain less or equal than 2 bytes should be skipped. Removing this check removes the need for a lot of special cases and fixes like mine (#4334). The need to call shall_skip in create_package_file (apidoc.py) might still need to remain though, to make TOC and modules generated consistent when using the option to exclude files.

So, just skipping the and path.getsize(module) <= 2

https://github.com/sphinx-doc/sphinx/blob/master/sphinx/ext/apidoc.py#L198

and it would solve this issue.

And an addition, the exact example put forth in this bug will render an inconsistent TOC in 1.6.5, I think @codingjoe meant to add something like this too for the django commands example:

echo "foo" > parent/child/foo.py  # The addition

[codingjoe]

True, this is a more realistic example to reproduce the problem.

pip install django
django-admin startproject example .
mkdir -p example/management/commands
touch example/management/__init__.py
touch example/management/commands/__init__.py
echo '"""Docstring"""' > example/management/commands/my_customer_command.py

[codingjoe]

Ok, I will open a PR about it. I don't know if I can finish it today, but I'll try to get it done EOW.

[zufallsgenerator]

Actually, it's a bit more complicated than I thought, because of the new feature with wildcard excludes (#930) that has been added since #4334 .

[zufallsgenerator]

I am working on a fix. Will push today.

[zufallsgenerator]

(Pull request)(#4670) added, should probably go to master and then 1.7.1
@codingjoe I think I've solved it already, so don't put unnecessary work in if you find my solution acceptable.

[codingjoe]

@tk0miya can we move this ahead? I am starting to freeze sphinx to 1.6.5 in more and more of my packages because people complain about missing documentation.

[tk0miya]

Fixed by #4670 and #4766.
Thanks,