Adi Roiban <email address hidden> writes:

> Adi Roiban has proposed merging lp:~adiroiban/pydoctor/1318325-intersphinx into lp:pydoctor.
>
> Requested reviews:
>   Michael Hudson-Doyle (mwhudson)
> Related bugs:
>   Bug #1318325 in pydoctor: "Support linking to interlinks objects.inv"
>   https://bugs.launchpad.net/pydoctor/+bug/1318325
>
> For more details, see:
> https://code.launchpad.net/~adiroiban/pydoctor/1318325-intersphinx/+merge/219128
>
> Problem description
> -------------------
>
> pydoctor has an hardcoded links for stdlibs but it would be nice to be able
> to link to sphinx apidocs using Sphinx inventory files

Heh, more amazingness!

>
> Changes
> -------
>
> Added '--intersphinx' configuration option.
>
> It assumes that a remote object inventory contains indexed for a single root package.
> This is why sphinx:http://domain.tld/latest/objects.inv will using 'http://domain.tld/latest/objects.inv' for any link starting with 'sphinx'
>
> System was updated to allow initialization with options and to download and parse requested objects inventory.
>
> Intersphinx inventories are stored using a simple dict and they provide a getLink method.
>
> epydoc2stan linker was updated to look for intersphinx links when previous lookups fail.

I guess in the fullness of time it would be nice to:

 a) support caching the objects.inv so that doc generation doesn't have
    to download it each time.
 b) allow the project being documented to define which objects.inv to use

neither of these feel at all urgent though.

> How to test
> -----------
>
> Check that README documentation make sense... not sure where to put this
> info.
>
> I have also done a simple manual test using sphinx object.inv from rtd.org
>
> Set one of the following markups
>
> def getparser():
>     """
>     Return parser for command line options.
>
>     L{pretty <sphinx.addnodes.desc_optional>}
>     """
>
> or 
>
> def getparser():
>     """
>     Return parser for command line options.
>
>     L{sphinx.addnodes.desc_optional}
>     """
>
> build pydoctor using
>
> pydoctor --intersphinx=sphinx:http://sphinx.readthedocs.org/en/latest/objects.inv pydoctor
>
> I tried to write as much test as possible but this is a weekend hack and there might be gaps in coverage.. or errors in code.
>
> Please check that changes make sense.
> Let me know what needs to be changes or if you have better idea for how
> this could be implemented.
>
> I don't have to much experience with pydoctor usage and how people expect
> to link to 3rd party.
>
> Thanks!
>
> -- 
> https://code.launchpad.net/~adiroiban/pydoctor/1318325-intersphinx/+merge/219128
> You are requested to review the proposed merge of lp:~adiroiban/pydoctor/1318325-intersphinx into lp:pydoctor.
> === modified file 'README.txt'
> --- README.txt	2013-05-29 03:28:59 +0000
> +++ README.txt	2014-05-12 01:08:05 +0000
> @@ -14,3 +14,20 @@
>  The default HTML generator requires Twisted.
>  
>  There are some more notes in the doc/ subdirectory.
> +
> +
> +Sphinx Integration
> +------------------
> +
> +It can link to external API documentation using Sphinx objects inventory using
> +the following cumulative configuration option::
> +
> +    --intersphinx=sphinx:http://sphinx.org/objects.inv
> +
> +All links starting with `sphinx`, (ex: `sphinx.sphinx.addnodes.desc_optional`)
> +are resolved based on Sphinx index from http://sphinx.org/objects.inv.
> +
> +It assumes that an inventory contains references for a single root package.
> +
> +You can add multiple intersphinx options to configure multiple external
> +project or inventories containing multiple root packages.
>
> === modified file 'pydoctor/driver.py'
> --- pydoctor/driver.py	2013-05-29 22:51:44 +0000
> +++ pydoctor/driver.py	2014-05-12 01:08:05 +0000
> @@ -185,6 +185,14 @@
>      parser.add_option(
>          '--introspect-c-modules', default=False, action='store_true',
>          help=("Import and introspect any C modules found."))
> +
> +    parser.add_option(
> +        '--intersphinx', action='append', dest='intersphinx',
> +        metavar='PACKAGE:URL_TO_OBJECTS.INV', default=[],
> +        help=(
> +            "Use Sphinx objects inventory for link generation for members"
> +            "from PACKAGE. Can be repeated."))
> +
>      return parser
>  
>  def readConfigFile(options):
> @@ -251,7 +259,10 @@
>          else:
>              system = systemclass()
>  
> +        # Once pickle support is removed, always instantiate System with
> +        # options and make fetchIntersphinxInventories private in __init__.
>          system.options = options
> +        system.fetchIntersphinxInventories()
>  
>          system.urlprefix = ''
>          if options.moresystems:
>
> === modified file 'pydoctor/epydoc2stan.py'
> --- pydoctor/epydoc2stan.py	2014-01-06 20:37:07 +0000
> +++ pydoctor/epydoc2stan.py	2014-05-12 01:08:05 +0000
> @@ -113,6 +113,18 @@
>                  thresh=-1)
>          return None
>  
> +    def look_for_intersphinx(self, name):
> +        """
> +        Return link for `name` based on intersphinx inventory.
> +
> +        Return None if link is not found.
> +        """
> +        base = name.split('.', 1)[0]
> +        inventory = self.obj.system.intersphinx.get(base, None)
> +        if not inventory:
> +            return None
> +        return inventory.getLink(name)
> +
>      def translate_identifier_xref(self, fullID, prettyID):
>          """Figure out what ``L{fullID}`` should link to.
>  
> @@ -163,6 +175,11 @@
>              self.obj.system.objectsOfType(model.Package)))
>          if target is not None:
>              return self._objLink(target, prettyID)
> +
> +        target = self.look_for_intersphinx(fullID)
> +        if target:
> +            return '<a href="%s"><code>%s</code></a>'%(target, prettyID)

Do you know if there is some way we can use
https://docs.python.org/2/objects.inv to replace the stdlib special
casing we have today?  I guess that's some should be some kind of
"inventory of last resort", to be consulted when none of the other
inventories find anything.

>          self.obj.system.msg(
>              "translate_identifier_xref", "%s:%s invalid ref to %s" % (
>                  self.obj.fullName(), self.obj.linenumber, fullID),
>
> === modified file 'pydoctor/model.py'
> --- pydoctor/model.py	2013-08-06 01:23:30 +0000
> +++ pydoctor/model.py	2014-05-12 01:08:05 +0000
> @@ -14,6 +14,8 @@
>  import types
>  import __builtin__
>  
> +from pydoctor.sphinx import SphinxInventory
> +
>  # originally when I started to write pydoctor I had this idea of a big
>  # tree of Documentables arranged in an almost arbitrary tree.
>  #
> @@ -347,7 +349,7 @@
>      #defaultBuilder = astbuilder.ASTBuilder
>      sourcebase = None
>  
> -    def __init__(self):
> +    def __init__(self, options=None):
>          self.allobjects = {}
>          self.orderedallobjects = []
>          self.rootobjects = []
> @@ -356,9 +358,14 @@
>          self.moresystems = []
>          self.subsystems = []
>          self.urlprefix = ''
> -        from pydoctor.driver import parse_args
> -        self.options, _ = parse_args([])
> -        self.options.verbosity = 3
> +
> +        if options:
> +            self.options = options
> +        else:
> +            from pydoctor.driver import parse_args
> +            self.options, _ = parse_args([])
> +            self.options.verbosity = 3

This is part of the same hack mentioned above right?  We can kill it
when pickle support dies.

>          self.abbrevmapping = {}
>          self.guessedprojectname = 'my project'
>          self.epytextproblems = [] # fullNames of objects that failed to epytext properly
> @@ -369,6 +376,7 @@
>          self.module_count = 0
>          self.processing_modules = []
>          self.buildtime = datetime.datetime.now()
> +        self.intersphinx = {}
>  
>      def verbosity(self, section=None):
>          if isinstance(section, str):
> @@ -702,3 +710,31 @@
>          while self.unprocessed_modules:
>              mod = iter(self.unprocessed_modules).next()
>              self.processModule(mod)
> +
> +
> +    def fetchIntersphinxInventories(self):
> +        """
> +        Download and parse intersphinx inventories based on configuration.
> +        """
> +        self.intersphinx = {}
> +
> +        for option in self.options.intersphinx:
> +            parts = option.split(':', 1)
> +            if len(parts) != 2:
> +                self.msg(
> +                    'sphinx',
> +                    'invalid format for intersphinx %s' % (option,)
> +                    )
> +                continue
> +            target_name = parts[0]
> +            target_url = parts[1]
> +            inventory = self._makeSphinxInventory(target_name)
> +            inventory.load(target_url)
> +            self.intersphinx[target_name] = inventory
> +
> +
> +    def _makeSphinxInventory(self, name):
> +        """
> +        Helper to instantiate sphinx inventories.
> +        """
> +        return SphinxInventory(logger=self.msg, project_name=name)
>
> === added file 'pydoctor/sphinx.py'
> --- pydoctor/sphinx.py	1970-01-01 00:00:00 +0000
> +++ pydoctor/sphinx.py	2014-05-12 01:08:05 +0000
> @@ -0,0 +1,107 @@
> +"""
> +Support for Sphinx compatibility.
> +"""
> +from __future__ import absolute_import
> +import urllib2
> +import zlib
> +
> +
> +class SphinxInventory(object):
> +    """
> +    Sphinx inventory handler.
> +    """
> +
> +    version = (2, 0)
> +
> +    def __init__(self, logger, project_name):
> +        self.project_name = project_name
> +        self.msg = logger
> +        self._base_url = ''
> +        self._links = {}
> +
> +    def load(self, url):
> +        """
> +        Load inventory from URL.
> +        """
> +        parts = url.rsplit('/', 1)
> +        if len(parts) != 2:
> +            self.msg(
> +                'sphinx', 'Failed to get remote base url for %s' % (url,))
> +            return
> +
> +        self._base_url = parts[0]
> +
> +        data = self._getURL(url)
> +
> +        if not data:
> +            self.msg(
> +                'sphinx', 'Failed to get object inventory from %s' % (url, ))
> +            return
> +
> +        payload = self._getPayload(data)
> +        self._links = self._parseInventory(payload)
> +
> +    def _getURL(self, url):
> +        """
> +        Get content of URL.
> +
> +        This is a helper for testing.
> +        """
> +        try:
> +            response = urllib2.urlopen(url)
> +            return response.read()
> +        except:
> +            return None
> +
> +    def _getPayload(self, data):
> +        """
> +        Parse inventory and return clear text payload without comments.
> +        """
> +        payload = ''
> +        while True:
> +            parts = data.split('\n', 1)
> +            if len(parts) != 2:
> +                payload = data
> +                break
> +            if not parts[0].startswith('#'):
> +                payload = data
> +                break
> +            data = parts[1]
> +        try:
> +            return zlib.decompress(payload)
> +        except:
> +            self.msg(
> +                'sphinx',
> +                'Failed to uncompress inventory from %s' % (self._base_url, ),
> +                )
> +            return ''

I feel like this should check the version field in the .inv file...

> +    def _parseInventory(self, payload):
> +        """
> +        Parse clear text payload and load it into internal links database.
> +        """
> +        result = {}
> +        for line in payload.splitlines():
> +            parts = line.split(' ', 4)
> +            if len(parts) != 5:
> +                self.msg(
> +                    'sphinx',
> +                    'Failed to parse line "%s" for %s' % (line, self._base_url),
> +                    )
> +                continue
> +            result[parts[0]] = parts[3]
> +        return result
> +
> +    def getLink(self, name):
> +        """
> +        Return link for `name` or None if no link is found.
> +        """
> +        relative_link = self._links.get(name, None)
> +        if not relative_link:
> +            return None
> +
> +        # For links ending with $, replace it with full name.
> +        if relative_link.endswith('$'):
> +            relative_link = relative_link[:-1] + name
> +
> +        return '%s/%s' % (self._base_url, relative_link)
>
> === modified file 'pydoctor/test/test_epydoc2stan.py'
> --- pydoctor/test/test_epydoc2stan.py	2013-05-23 02:03:27 +0000
> +++ pydoctor/test/test_epydoc2stan.py	2014-05-12 01:08:05 +0000
> @@ -1,6 +1,9 @@
>  from pydoctor import epydoc2stan
> +from pydoctor import model
> +from pydoctor.sphinx import SphinxInventory
>  from pydoctor.test.test_astbuilder import fromText
>  
> +
>  def test_multiple_types():
>      mod = fromText('''
>      def f(a):
> @@ -64,3 +67,68 @@
>      assert u'Lorem Ipsum' == get_summary('single_line_summary')
>      assert u'Foo Bar Baz' == get_summary('three_lines_summary')
>      assert u'No summary' == get_summary('no_summary')
> +
> +
> +def test_EpydocLinker_look_for_intersphinx_no_base():
> +    """
> +    Return None if no inventory exists for link.
> +    """
> +    system = model.System()
> +    target = model.Module(system, 'ignore-name', 'ignore-docstring')
> +    sut = epydoc2stan._EpydocLinker(target)
> +
> +    result = sut.look_for_intersphinx('another.module')
> +
> +    assert None is result
> +
> +
> +def test_EpydocLinker_look_for_intersphinx_no_link():
> +    """
> +    Return None if inventory had no link for our markup.
> +    """
> +    system = model.System()
> +    system.intersphinx['base'] = SphinxInventory(system.msg, 'some-project')
> +    target = model.Module(system, 'ignore-name', 'ignore-docstring')
> +    sut = epydoc2stan._EpydocLinker(target)
> +
> +    result = sut.look_for_intersphinx('base.module')
> +
> +    assert None is result
> +
> +
> +def test_EpydocLinker_look_for_intersphinx_hit():
> +    """
> +    Return the link from inventory based on first package name.
> +    """
> +    system = model.System()
> +    inventory = SphinxInventory(system.msg, 'some-project')
> +    inventory._base_url = 'http://tm.tld'
> +    inventory._links['base.module.other'] = 'some.html'
> +    system.intersphinx['base'] = inventory
> +    target = model.Module(system, 'ignore-name', 'ignore-docstring')
> +    sut = epydoc2stan._EpydocLinker(target)
> +
> +    result = sut.look_for_intersphinx('base.module.other')
> +
> +    assert 'http://tm.tld/some.html' == result
> +
> +
> +def test_EpydocLinker_translate_identifier_xref_intersphinx():
> +    """
> +    Return the link from inventory.
> +    """
> +    system = model.System()
> +    inventory = SphinxInventory(system.msg, 'some-project')
> +    inventory._base_url = 'http://tm.tld'
> +    inventory._links['base.module.other'] = 'some.html'
> +    system.intersphinx['base'] = inventory
> +    target = model.Module(system, 'ignore-name', 'ignore-docstring')
> +    sut = epydoc2stan._EpydocLinker(target)
> +
> +    result = sut.translate_identifier_xref(
> +        'base.module.other', 'base.module.pretty')
> +
> +    expected = (
> +        '<a href="http://tm.tld/some.html"><code>base.module.pretty</code></a>'
> +        )
> +    assert expected == result
>
> === modified file 'pydoctor/test/test_model.py'
> --- pydoctor/test/test_model.py	2008-03-16 19:09:50 +0000
> +++ pydoctor/test/test_model.py	2014-05-12 01:08:05 +0000
> @@ -1,4 +1,11 @@
> +"""
> +Unit tests for model.
> +"""
>  from pydoctor import model
> +from pydoctor.driver import parse_args
> +from pydoctor.sphinx import SphinxInventory
> +import zlib
> +
>  
>  class FakeOptions(object):
>      """
> @@ -41,3 +48,81 @@
>  
>      expected = viewSourceBase + moduleRelativePart
>      assert mod.sourceHref == expected
> +
> +
> +def test_initialization_default():
> +    """
> +    When initialized without options, will use default options and default
> +    verbosity.
> +    """
> +    sut = model.System()
> +
> +    assert None is sut.options.projectname
> +    assert 3 == sut.options.verbosity
> +
> +
> +def test_initialization_options():
> +    """
> +    Can be initialized with options.
> +    """
> +    options = object()
> +
> +    sut = model.System(options=options)
> +
> +    assert options is sut.options
> +
> +
> +def test_fetchIntersphinxInventories_empty():
> +    """
> +    Convert option to empty dict.
> +    """
> +    options, _ = parse_args([])
> +    options.intersphinx = []
> +    sut = model.System(options=options)
> +
> +    sut.fetchIntersphinxInventories()
> +
> +    assert {} == sut.intersphinx
> +
> +
> +def test_fetchIntersphinxInventories_content():
> +    """
> +    Download and parse intersphinx inventories for each configured
> +    intersphix.
> +    """
> +    options, _ = parse_args([])
> +    options.intersphinx = [
> +        'sphinx:http://sphinx/objects.inv',
> +        'twisted:file:///twisted/index.inv',
> +        ]
> +    url_content = {
> +        'http://sphinx/objects.inv': zlib.compress(
> +            'sphinx.module py:module -1 sp.html -'),
> +        'file:///twisted/index.inv': zlib.compress(
> +            'twisted.package py:module -1 tm.html -'),
> +        }
> +    sut = model.System(options=options)
> +    log = []
> +    sut.msg = lambda part, msg: log.append((part, msg))
> +
> +    def patchSphinxInventory(name):
> +        """
> +        Patch url getter to avoid touching the network.
> +        """
> +        inventory = SphinxInventory(logger=sut.msg, project_name=name)
> +        inventory._getURL = lambda url: url_content[url]
> +        return inventory
> +    sut._makeSphinxInventory = patchSphinxInventory
> +
> +    sut.fetchIntersphinxInventories()
> +
> +    assert 2 == len(sut.intersphinx)
> +    assert [] == log
> +    assert (
> +        'http://sphinx/sp.html' ==
> +        sut.intersphinx['sphinx'].getLink('sphinx.module')
> +        )
> +    assert (
> +        'file:///twisted/tm.html' ==
> +        sut.intersphinx['twisted'].getLink('twisted.package')
> +        )
>
> === added file 'pydoctor/test/test_sphinx.py'
> --- pydoctor/test/test_sphinx.py	1970-01-01 00:00:00 +0000
> +++ pydoctor/test/test_sphinx.py	2014-05-12 01:08:05 +0000
> @@ -0,0 +1,211 @@
> +"""
> +Tests for Sphinx integration.
> +"""
> +import zlib
> +
> +from pydoctor.sphinx import SphinxInventory
> +
> +
> +def make_SphinxInventory():
> +    """
> +    Return a SphinxInventory.
> +    """
> +    return SphinxInventory(logger=object(), project_name='project_name')
> +
> +
> +def make_SphinxInventoryWithLog():
> +    """
> +    Return a SphinxInventory with patched log.
> +    """
> +    inventory = make_SphinxInventory()
> +    log = []
> +    inventory.msg = lambda part, msg: log.append((part, msg))
> +    return (inventory, log)
> +
> +
> +def test_getPayload_empty():
> +    """
> +    Return empty string.
> +    """
> +    sut = make_SphinxInventory()
> +    content = """# Sphinx inventory version 2
> +# Project: some-name
> +# Version: 2.0
> +# The rest of this file is compressed with zlib.
> +x\x9c\x03\x00\x00\x00\x00\x01"""
> +
> +    result = sut._getPayload(content)
> +
> +    assert '' == result
> +
> +
> +def test_getPayload_content():
> +    """
> +    Return content as string.
> +    """
> +    payload = 'first_line\nsecond line'
> +    sut = make_SphinxInventory()
> +    content = """# Ignored line
> +# Project: some-name
> +# Version: 2.0
> +# commented line.
> +%s""" % (zlib.compress(payload),)
> +
> +    result = sut._getPayload(content)
> +
> +    assert payload == result
> +
> +
> +def test_getPayload_invalid():
> +    """
> +    Return empty string and log an error when failing to uncompress data.
> +    """
> +    sut, log = make_SphinxInventoryWithLog()
> +    sut._base_url = 'http://tm.tld'
> +    content = """# Project: some-name
> +# Version: 2.0
> +not-valid-zlib-content"""
> +
> +    result = sut._getPayload(content)
> +
> +    assert '' == result
> +    assert [(
> +        'sphinx', 'Failed to uncompress inventory from http://tm.tld',
> +        )] == log
> +
> +
> +def test_getLink_not_found():
> +    """
> +    Return None if link does not exists.
> +    """
> +    sut = make_SphinxInventory()
> +
> +    assert None is sut.getLink('no.such.name')
> +
> +
> +def test_getLink_found():
> +    """
> +    Return the link from internal state.
> +    """
> +    sut = make_SphinxInventory()
> +    sut._base_url = 'http://base.tld'
> +    sut._links['some.name'] = 'some/url.php'
> +
> +    assert 'http://base.tld/some/url.php' == sut.getLink('some.name')
> +
> +
> +def test_getLink_self_anchor():
> +    """
> +    Return the link with anchor as target name when link end with $.
> +    """
> +    sut = make_SphinxInventory()
> +    sut._base_url = 'http://base.tld'
> +    sut._links['some.name'] = 'some/url.php#$'
> +
> +    assert 'http://base.tld/some/url.php#some.name' == sut.getLink('some.name')
> +
> +
> +def test_load_functional():
> +    """
> +    Functional test for loading an empty inventory.
> +    """
> +    payload = (
> +        'some.module1 py:module -1 module1.html -\n'
> +        'other.module2 py:module 0 module2.html Other description\n'
> +        )
> +    sut = make_SphinxInventory()
> +    # Patch URL loader to avoid hitting the system.
> +    content = """# Sphinx inventory version 2
> +# Project: some-name
> +# Version: 2.0
> +# The rest of this file is compressed with zlib.
> +%s""" % (zlib.compress(payload),)
> +    sut._getURL = lambda _: content
> +
> +    sut.load('http://some.url/api/objects.inv')
> +
> +    assert 'http://some.url/api/module1.html' == sut.getLink('some.module1')
> +    assert 'http://some.url/api/module2.html' == sut.getLink('other.module2')
> +
> +
> +def test_load_bad_url():
> +    """
> +    Log an error when failing to get base url from url.
> +    """
> +    sut, log = make_SphinxInventoryWithLog()
> +
> +    sut.load('really.bad.url')
> +
> +    assert sut._links == {}
> +    expected_log = [(
> +        'sphinx', 'Failed to get remote base url for really.bad.url'
> +        )]
> +    assert expected_log == log
> +
> +
> +def test_load_fail():
> +    """
> +    Log an error when failing to get content from url.
> +    """
> +    sut, log = make_SphinxInventoryWithLog()
> +    sut._getURL = lambda _: None
> +
> +    sut.load('http://some.tld/o.inv')
> +
> +    assert sut._links == {}
> +    expected_log = [(
> +        'sphinx', 'Failed to get object inventory from http://some.tld/o.inv'
> +        )]
> +    assert expected_log == log
> +
> +
> +def test_parseInventory_empty():
> +    """
> +    Return empty dict for empty input.
> +    """
> +    sut = make_SphinxInventory()
> +
> +    result = sut._parseInventory('')
> +
> +    assert {} == result
> +
> +
> +def test_parseInventory_single_line():
> +    """
> +    Return a dict with a single member.
> +    """
> +    sut = make_SphinxInventory()
> +
> +    result = sut._parseInventory('some.attr py:attr -1 some.html De scription')
> +
> +    assert {'some.attr': 'some.html'} == result
> +
> +
> +def test_parseInventory_invalid_lines():
> +    """
> +    Skip line and log an error.
> +    """
> +    sut, log = make_SphinxInventoryWithLog()
> +    sut._base_url = 'http://tm.tld'
> +    content = (
> +        'good.attr py:attribute -1 some.html -\n'
> +        'bad.attr bad format\n'
> +        'very.bad\n'
> +        '\n'
> +        'good.again py:module 0 again.html -\n'
> +        )
> +
> +    result = sut._parseInventory(content)
> +
> +    assert {
> +        'good.attr': 'some.html',
> +        'good.again': 'again.html'
> +        } == result
> +    assert [
> +        (
> +            'sphinx',
> +            'Failed to parse line "bad.attr bad format" for http://tm.tld'
> +            ),
> +        ('sphinx', 'Failed to parse line "very.bad" for http://tm.tld'),
> +        ('sphinx', 'Failed to parse line "" for http://tm.tld'),
> +        ] == log

Cheers,
mwh