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

Documentation issues with intersphinx #379

Open
valschneider opened this issue Mar 20, 2019 · 10 comments
Open

Documentation issues with intersphinx #379

valschneider opened this issue Mar 20, 2019 · 10 comments

Comments

@valschneider
Copy link
Contributor

Hi,

I thought I had already raised an issue for this, but just realized that was not the case.

We've been busy writing a shiny new documentation for LISA, but for some reason references to devlib (and workload-automation) fail to be resolved:

lisa/doc/target.rst:8: WARNING: py:class reference target not found: devlib.target.Target

(https://my-lisa.readthedocs.io/en/lisa-next/target.html#introduction)

For now we ignore sphinx build errors that mention devlib/wa, but it'd be nice to get these working. I don't really know why these two specific projects cause us problems though - trappy works fine for instance: https://my-lisa.readthedocs.io/en/lisa-next/trace_analysis.html#introduction

FWIW this is our intersphinx mapping:

intersphinx_mapping = {
    'python' : ('https://docs.python.org/3', None),
    'pandas' : ('https://pandas.pydata.org/pandas-docs/stable/', None),
    'matplotlib' : ('https://matplotlib.org', None),
    # XXX: Doesn't seem to work, might be due to how devlib doc is generated
    'devlib' : ('https://pythonhosted.org/devlib/', None),
    'trappy' : ('https://pythonhosted.org/TRAPpy', None),
    'bart' :   ('https://pythonhosted.org/bart-py/', None),
    'wa' : ('https://pythonhosted.org/wlauto/', None),
}
@marcbonnici
Copy link
Collaborator

I can't say I have experimented with intersphinx before so might need to look into this in more detail, however one thing that stands out is this is using the (very) out of date pythonhosted versions of the documentation. Our current documentation are hosted at https://workload-automation.readthedocs.io/en/latest/ and https://devlib.readthedocs.io/en/latest/

Could you try updating your URLs and see if that solves the issue?

@valschneider
Copy link
Contributor Author

Our current documentation are hosted at https://workload-automation.readthedocs.io/en/latest/ and https://devlib.readthedocs.io/en/latest/

Could you try updating your URLs and see if that solves the issue?

Sadly that seems to give the same result :(

@marcbonnici
Copy link
Collaborator

Ok so looking at the exported objects from devlib documentaion with:
python -msphinx.ext.intersphinx https://devlib.readthedocs.io/en/latest/objects.inv

The Target class looks like it is exported however I'm wondering if the correct notation would be just devlib.Target?

@valschneider
Copy link
Contributor Author

Same result apparently:

WARNING: py:class reference target not found: devlib.Target

Looking at the objects, I wonder if the doc structure for devlib/wa isn't throwing sphinx off balance. For instance, using your command on trappy, I can see this:

py:class
	trappy.ftrace.FTrace                     trappy.ftrace.html#trappy.ftrace.FTrace

But for devlib it looks like this:

py:class
	Target                                   target.html#Target

The objects are all there, so there might be a way to tweak sphinx to correctly fetch them. I'll have a look.

@douglas-raillard-arm
Copy link
Collaborator

Had a quick look at it and the issue would likely be solved by adding .. module:: directives (with the full name, like devlib.target around the .. class::. Otherwise, autodoc (and then ..automodule::/ .. autoclass::/..autofunction::) can be used to parse the docstrings, and will generate good names, along with signature that are guaranteed to be in sync with the code.

@marcbonnici
Copy link
Collaborator

Thanks for finding that, I have tried adding module directives in my development version (https://mabonnici-testing-devlib.readthedocs.io/en/latest/index.html) of the docs. Could you try this url and see if this allows the references to work correctly?

@douglas-raillard-arm
Copy link
Collaborator

Hi Marc, this seems to be better, I've spotted some inconsistencies that once solved should give the proper full names:

  • In one place there are two module directives, which is likely to be the cause of devlib.BaylibreAcmeInstrument being registered rather than the expected devlib.instrument.baylibre_acme.BaylibreAcmeInstrument. There's also a typo: s/devlib.target/devlib.instrument/

.. module:: devlib.target.baylibre_acme

.. module:: devlib
   :noindex:
  • The Target classes also have the same issue (they are registered as devlib.Target rather than devlib.target.Target).

Other than that, classes like devlib.utils.ssh.SshConnection are indexed with the right full name so the .. module:: trick seems to be working :)

note: If you want short names in the doc, you can use :class:`~devlib.target.Target` rather than :class:`Target <devlib.target.Target>` , see https://www.sphinx-doc.org/en/master/usage/restructuredtext/domains.html#cross-referencing-syntax

@marcbonnici
Copy link
Collaborator

Hi Douglas,

I was originally hoping by using the two model directives we could export under both the long and shortened version of the names however that does not seem to be the case so have removed the duplicate so this should hopefully work correctly now.

And thanks for the headsup on the typos and the naming shortcut, I have updated the branch and dev version of the documentation accordingly.

If you get a chance to test this out / spot any more issues please let me know.

@douglas-raillard-arm
Copy link
Collaborator

douglas-raillard-arm commented Dec 27, 2019

Hi Marc,

The only remaining things are classes/modules that are simply not in the doc ATM, so the other issues seem fixed :)

One notable missing reference is a devlib module declaration (missing for LISA as well: ARM-software/lisa#1270). The rest seems to be stuff I introduced in devlib with docstrings like DmesgCollector. It would be easy to add .. autoclass directives to include that where it should in the doc, unless you prefer moving the docstring altogether. Autodoc has a number of advantages though:

  • Since the doc is written in the docstring, it becomes available with help() python builtin (and jupyter notebook help tooltip) and pydoc
  • The doc stays close to the code
  • Autodoc generates all the module/class/method/attribute declarations that will work out of the box with intersphinx
  • Autodoc generates the "prototype" of the function with parameter's default values, and provides a pretty nice parameter name/type/doc list
  • Since the auto* directives are explicitly written, it can be mixed with the existing doc without needing to convert everything

LISA is integrally documented using autodoc, e.g.:
HTML : https://lisa-linux-integrated-system-analysis.readthedocs.io/en/master/workloads.html
Source: https://raw.githubusercontent.com/ARM-software/lisa/master/doc/workloads.rst

You just need to enable that sphinx extension:
https://github.com/ARM-software/lisa/blob/master/doc/conf.py#L131
And (optionally) tweak some settings:
https://github.com/ARM-software/lisa/blob/master/doc/conf.py#L388

reference:
https://www.sphinx-doc.org/en/master/usage/extensions/autodoc.html

@marcbonnici
Copy link
Collaborator

Thanks for checking that out. I've created a PR with those changes (and the missing devlib module declaration) for here: #459

The remaining missing module/class documentation is going to be a larger change so will leave that for a later PR however appreciate the information and examples.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants