One issue and one suggestion.

Issue:

The documentation of prepare() says:

> If any cycle is detected, CycleError will be raised

which is what happens. The documentation of static_order() says that static_order() is equivalent to:

def static_order(self):
    self.prepare()
    while self.is_active():
        node_group = self.get_ready()
        yield from node_group
        self.done(*node_group)

specifically it is said to call self.prepare(), and also says

> If any cycle is detected, CycleError will be raised.

But, this only happens when the result of static_order is *iterated*, not when it's called, unlike what is suggested by the code and the comment.

Ideally, I think the call should raise the CycleError already if possible; this way, only the call can be wrapped in a try/except instead of the entire iteration. But if not, it should be clarified in the documentation.


Suggestion:

The documentation of static_order() says

> Returns an iterable of nodes in a topological order. Using this method does not require to call TopologicalSorter.prepare() or TopologicalSorter.done().

I think the wording "does not require" still implies that they *can* be called, but really they can't. If prepare() is called before static_order(), then when static_order() is iterated, "ValueError: cannot prepare() more than once" is raised.

I suggest this wording:

Returns an iterable of nodes in a topological order. When using this method, TopologicalSorter.prepare() and TopologicalSorter.done() should not be called.

Hmm I realize after the fact that since the equivalent code snippet (which is actually the implementation) is a generator, then it is actually correct that prepare() isn't called until it is iterated. So the documentation for this is not incorrect.

Still, I think it would be better if the prepare() executes immediately rather than in the first iteration.

Hi Ran, would you like to submit a pull request with the suggestion?

I'm not sure it's worth it to complicate the logic by raising the error before iteration since iteration is normally done soon after method call.

I agree the doc change is helpful, since the ticket was dormant for a ~month, I've put up a PR with this change as well as clarifying that an iterator obj is returned rather than an iterable.

I've given credit to Ran in the PR description.

New changeset 0d7f7975d55eff7e3dfcebd14e765fc6cd7d3e40 by andrei kulakov in branch 'main':
bpo-42588: Update the docs for the TopologicalSorter.static_order() method (GH-26834)
https://github.com/python/cpython/commit/0d7f7975d55eff7e3dfcebd14e765fc6cd7d3e40

Thanks, Ran and Andrei for your contributions! :)

New changeset d9fc4c3deb617beb1d0bbfdb4efc905a4192ff0a by Miss Islington (bot) in branch '3.10':
bpo-42588: Update the docs for the TopologicalSorter.static_order() method (GH-26834)
https://github.com/python/cpython/commit/d9fc4c3deb617beb1d0bbfdb4efc905a4192ff0a

New changeset 3ba65cdcefcec7866be482138a5ea8aafbd07e59 by Miss Islington (bot) in branch '3.9':
bpo-42588: Update the docs for the TopologicalSorter.static_order() method (GH-26834) (GH-26952)
https://github.com/python/cpython/commit/3ba65cdcefcec7866be482138a5ea8aafbd07e59