Skip to content

Conversation

@mkelley
Copy link

@mkelley mkelley commented Jan 2, 2026

Following #219, this PR adds tests for two directives in series. For example:

.. doctest-requires:: numpy<=0.1
.. deprecated:: 1.0

    >>> 1 + 3
    2

Currently this example will fail building with Sphinx with the same error as shown in #219 (i.e., it was not fixed by #311). The error arises because .. doctest-requires in the above example has no content following it, so indexing self.content in its run() fails:

class DoctestSkipDirective(Directive):
    has_content = True

    def run(self):
        # Check if there is any valid argument, and skip it. Currently only
        # 'win32' is supported.
        if re.match('win32', self.content[0]):
            self.content = self.content[2:]
        code = '\n'.join(self.content)
        return [literal_block(code, code)]

In this PR, I've added a test in the if statement to first require len(self.content) > 0 before testing for "win32", but the rendered documentation does not look good. There are empty code blocks added for each "empty" doctestplus directive:

image

Recognizing that Sphinx directives in series do have have this problem, I tried sub-classing from SphinxDirective and the result looks good:

image

I don't know if you want to take that direction, but I figured that this PR could at least show a possible solution.

When there are two directives in series, the first one will not have
any content.  As a result, self.content[0] will raise an error.
DoctestSkipDirective inherits from SphinxDirective, which does not
render empty blocks when content is empty.
@pllim pllim requested review from bsipocz and seberg January 2, 2026 15:12
@pllim pllim added the bug label Jan 2, 2026
@pllim pllim added this to the v1.6.1 milestone Jan 2, 2026
@pllim
Copy link
Contributor

pllim commented Jan 2, 2026

Looks pretty clean but I'll give others a chance to review. Thanks!

Could use change log in CHANGES.rst.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants