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

Support docutils-0.18: Consume iterator of Element.traverse() #9780

Merged
merged 1 commit into from
Oct 26, 2021

Conversation

tk0miya
Copy link
Member

@tk0miya tk0miya commented Oct 26, 2021

Feature or Bugfix

  • Refactoring

Purpose

  • Since 0.18, Element.traverse() returns an iterator instead of
    intermediate object. As a result, the return value is always considered
    as truthy value. And it becomes fragile when the caller modifies the
    doctree on the loop.
  • refs: Update Sphinx to work with docutils-0.18.x #9777

Since 0.18, Element.traverse() returns an iterator instead of
intermediate object.  As a result, the return value is always considered
as truthy value.  And it becomes fragile when the caller modifies the
doctree on the loop.
@tk0miya tk0miya added internals:refactoring docutils Tags upstream Docutils issues labels Oct 26, 2021
@tk0miya tk0miya added this to the 4.3.0 milestone Oct 26, 2021
@tk0miya tk0miya merged commit e719b82 into sphinx-doc:4.x Oct 26, 2021
@tk0miya tk0miya deleted the docutils-0.18_traverse branch October 26, 2021 17:02
@gmilde
Copy link

gmilde commented Nov 8, 2021

The reason for returning an iterator instead of a list is a considerable speedup in the majority of use cases. https://sourceforge.net/p/docutils/patches/160/
Be carful not to waste this gain by wrapping the return value in a list unless required.

If I get this right,

        doc = tree.traverse(nodes.document)[0]

just wants the first element that is of type "nodes.document".
Instead of traversing the complete tree, building a list of "document" nodes and picking the first

        doc = list(tree.traverse(nodes.document))[0]

one could use the iterators "next()" method:

        doc = tree.traverse(nodes.document).next()

(In cases where no matching node exists, the error returned would be
StopIteration instead of IndexError.)

or even simpler the next_node() method:

        doc = tree.next_node(nodes.document).next()

which returns the first matching node or None. (Note the different default of the optional argument "include_self".)

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
docutils Tags upstream Docutils issues internals:refactoring
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants