sphinx-doc / sphinx

The Sphinx documentation generator
https://www.sphinx-doc.org/
Other
6.43k stars 2.1k forks source link

Autosummary 'attributes' template variable excludes some module attributes #8182

Open dfremont opened 4 years ago

dfremont commented 4 years ago

Describe the bug Commit 784d4cb36a2aca4bcb2c773db2506cc609a2a18a broke autosummary's computation of module attributes (added in #7469). Specifically, the attributes template variable now wrongly excludes any module attribute whose value is an object created in another module.

To Reproduce Apply autosummary to a module with a documented attribute whose value is an instance of a class defined in another module. The attribute will not be listed in the generated documentation (see example project below).

Expected behavior The attributes template variable should include all documented attributes defined in the module (there is no question of trying to hide "imported members" here, since if you import a name at module-level then it will not have a docstring or doc-comment).

Your project Here is a tiny project triggering the bug (testA.thing1 is properly documented, but the entry for testA.thing2 is missing): example2.zip

Environment info

Additional context The error was introduced by commit 784d4cb36a2aca4bcb2c773db2506cc609a2a18a: when autosummary.generate.generate_autosummary_content calls get_module_attrs, it passes in the list of members ns['members']. This used to be the same as dir(obj), but now uses ModuleScanner to exclude imported members. ModuleScanner introspects the value of the member to check whether it was defined elsewhere, which doesn't make sense for module-level attributes. You can fix the bug by simply calling get_module_attrs with dir(obj).

dfremont commented 4 years ago

Oops, my suggested fix will correct the attributes template variable, but the members variable will still omit the attributes in question. I guess a better approach would be to call get_module_attrs first to find all module attributes, and then have ModuleScanner add to the resulting list (being careful not to create duplicates). I can put together a PR if it would be helpful, although I don't think I can add a test case - multiple autosummary tests are already failing on my machine for some reason.

fritzr commented 3 years ago

Oops, my suggested fix will correct the attributes template variable, but the members variable will still omit the attributes in question. I guess a better approach would be to call get_module_attrs first to find all module attributes, and then have ModuleScanner add to the resulting list (being careful not to create duplicates). [...]

Once upon a time I patched this locally and I basically did this. Call get_module_attrs() on dir(obj), which internally accepts items where the documenter's objtype (or directivetype if present) is "attribute", or if objtype/directivetype is "data" and the object name is in the analyzer's tagorder or find_attr_docs().

I'd love to see this patched before 4.x becomes more widespread. The main reason I had a local patch of autosummary in the first place was to manually implement the :recursive: option back in 1.x. I was excited to drop my local patch with the introduction of :recursive: in 3.1, but this bug hides a lot of documentation when using :recursive: in 4.0. I don't see a good workaround that doesn't involve patching autosummary. (If I get some spare time I'll submit a PR.)