Fix: no links generated in summary table

[theOehrly]

This pull request fixes #65 and #67.

Description

In some specific cases, dependent on the import structure of a project, summary tables generated by autodocsumm do not contain hyperlinks to the referenced objects. This only happens if imports of top level modules allow a "circular" reference to an object or module (such a reference is not necessarily a valid import path). Assuming the following project structure

my_module
├─ submodule1
├─ submodule2

the problem occurs if submodule1.py contains an import that exposes the namespace of my_module. For example import my_module, import my_module.submodule2 and variations thereof. This theoretically allows to reference my_module.submodule1 as my_module.submodule1.my_module.submodule1.

Furthermore, the problem only occurs, when the :autosummary: option is used within a .. automodule:: or .. autoclass:: directive.

The problem does not occur when using .. autoclasssumm, .. automodulesumm.

#67 provides a minimal example to reproduce the problem.

Solution

The reason for this problem is given in sphinx-doc/sphinx#6792 (comment)

The .. automodule:: and .. autoclass:: directives use the :module: option to modify the current module. Any .. autosummary:: directive added inside one of these directives should therefore reference objects relative to the 'current module'.

At the moment, the :autosummary: option generates the following intermediary output:

.. py:class:: MyClass()
   :module: dummy.submodule1
   Docu for my class

   **Methods:**

   .. autosummary::
       ~dummy.submodule1.MyClass.func1

This pull requests changes the behaviour, so that the output uses relative references:

.. py:class:: MyClass()
   :module: dummy.submodule1
   Docu for my class

   **Methods:**

   .. autosummary::
       ~MyClass.func1

Note

It is actually not the case, that the output generated by autodocsumm stops working in case of a specific import structure. The objects should always be referenced relative to the current module. Therefore, it is the case that the output sometimes works, despite being incorrect.

Implementation

Sphinx' autodoc sets :module: to self.modname. See:

https://github.com/sphinx-doc/sphinx/blob/40a8f2b54a04872fea24d316baf0c39066897c2d/sphinx/ext/autodoc/__init__.py#L546-L548

To correct for this, the fix in this PR strips self.modname from the beginning of the reference. No check is done to ensure that the result is a valid reference path. Tests have shown that this is sufficient. Verifying the resulting import path will certainly be slower, but could be done if necessary.

The change is implemented so that .add_autosummary defaults to the old behaviour of using absolute references. This is required for other directives.

Tests

Test have been added. I'm not familiar with the sphinx testing framework used here, therefore, some changes might be necessary.

I have also built the documentation of two larger projects of mine with this change. All missing links in summary tables were added correctly, and I could not spot any other problems.

[Chilipp]

Great! Thanks a lot @theOehrly !

[Chilipp]

The CI fails but I assume it's because of changes in Sphinx. I can have a look at it tomorrow

[Chilipp]

hey @theOehrly! thanks again a lot for your contribution! My apologies for taking so long, I have just too much to do at the moment. But I will fix this by the end of this week, just to let you know that I did not forget about it.

[theOehrly]

@Chilipp I can rebase this onto master again if you are happy to merge #71 to fix the CI

[Chilipp]

Ah! Now I indeed forgot about it, my sincere apologies. Thanks a lot @theOehrly ! Let's do it that way.

[codecov]

Codecov Report

Merging #70 (93a7473) into master (073d513) will increase coverage by 0.21%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master      #70      +/-   ##
==========================================
+ Coverage   86.46%   86.68%   +0.21%     
==========================================
  Files           1        1              
  Lines         303      308       +5     
==========================================
+ Hits          262      267       +5     
  Misses         41       41              

Impacted Files	Coverage Δ
autodocsumm/__init__.py	86.68% <100.00%> (+0.21%)	⬆️

---

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 073d513...93a7473. Read the comment docs.

[theOehrly]

@Chilipp so CI is still failing. I'll get back to you once I have it figured out.

[Chilipp]

Hm, maybe we should just drop support for sphinx<3.5. I feel like maintaining support for the different sphinx versions is most of the work in this package...

[theOehrly]

I guess the impact of that would be small. But let me check first, what the problems are and how much work is necessary to get it running with all currently supported versions.

[Chilipp]

I just tested it locally @theOehrly and the problem exists indeed for sphinx>=3.5, too. It's just that the tests are not realising it because Sphinx now also inserts <span class="pre">test_func</span> elements outside of the autosummary. I need to change the test strategy here

[Chilipp]

hey @theOehrly! I just fixed the test method in #72 and rebased this PR to implement the tests

[theOehrly]

Ok, will take a look at the failures some time this week

[Chilipp]

I can have a look into it now if you like.

[theOehrly]

Sure, if you have time. You'll very likely understand the failing tests better than I do right now. I'd have to figure out what actually is being tested there first.

[Chilipp]

alright, let's merge this! thanks again @theOehrly for the contribution

[theOehrly]

Awesome, thank you for helping with getting this done