Closed theOehrly closed 2 years ago
Great! Thanks a lot @theOehrly !
The CI fails but I assume it's because of changes in Sphinx. I can have a look at it tomorrow
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.
@Chilipp I can rebase this onto master again if you are happy to merge #71 to fix the CI
Ah! Now I indeed forgot about it, my sincere apologies. Thanks a lot @theOehrly ! Let's do it that way.
Merging #70 (93a7473) into master (073d513) will increase coverage by
0.21%
. The diff coverage is100.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%) |
:arrow_up: |
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.
@Chilipp so CI is still failing. I'll get back to you once I have it figured out.
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...
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.
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
hey @theOehrly! I just fixed the test method in #72 and rebased this PR to implement the tests
Ok, will take a look at the failures some time this week
I can have a look into it now if you like.
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.
alright, let's merge this! thanks again @theOehrly for the contribution
Awesome, thank you for helping with getting this done
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
the problem occurs if
submodule1.py
contains an import that exposes the namespace ofmy_module
. For exampleimport my_module
,import my_module.submodule2
and variations thereof. This theoretically allows to referencemy_module.submodule1
asmy_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 https://github.com/sphinx-doc/sphinx/pull/6792#issuecomment-549833172
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:This pull requests changes the behaviour, so that the output uses relative references:
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:
toself.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.