[issue42588] Improvements to graphlib.TopologicalSorter.static_order() documentation

[Pablo Galindo Salgado]

Pablo Galindo Salgado  added the comment:


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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42588] Improvements to graphlib.TopologicalSorter.static_order() documentation

[miss-islington]

miss-islington  added the comment:


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


--
nosy: +miss-islington

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42588] Improvements to graphlib.TopologicalSorter.static_order() documentation

[Pablo Galindo Salgado]

Pablo Galindo Salgado  added the comment:

Thanks, Ran and Andrei for your contributions! :)

--
nosy:  -miss-islington
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42588] Improvements to graphlib.TopologicalSorter.static_order() documentation

[miss-islington]

Change by miss-islington :


--
nosy: +miss-islington
nosy_count: 6.0 -> 7.0
pull_requests: +25519
pull_request: https://github.com/python/cpython/pull/26951

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42588] Improvements to graphlib.TopologicalSorter.static_order() documentation

[miss-islington]

Change by miss-islington :


--
pull_requests: +25520
pull_request: https://github.com/python/cpython/pull/26952

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42588] Improvements to graphlib.TopologicalSorter.static_order() documentation

[Pablo Galindo Salgado]

Pablo Galindo Salgado  added the comment:


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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42588] Improvements to graphlib.TopologicalSorter.static_order() documentation

[Andrei Kulakov]

Andrei Kulakov  added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42588] Improvements to graphlib.TopologicalSorter.static_order() documentation

[Andrei Kulakov]

Change by Andrei Kulakov :


--
keywords: +patch
nosy: +andrei.avk
nosy_count: 5.0 -> 6.0
pull_requests: +25415
stage:  -> patch review
pull_request: https://github.com/python/cpython/pull/26834

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42588] Improvements to graphlib.TopologicalSorter.static_order() documentation

[Pablo Galindo Salgado]

Pablo Galindo Salgado  added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42588] Improvements to graphlib.TopologicalSorter.static_order() documentation

[Karthikeyan Singaravelan]

Change by Karthikeyan Singaravelan :


--
nosy: +pablogsal, rhettinger, tim.peters

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42588] Improvements to graphlib.TopologicalSorter.static_order() documentation

[Ran Benita]

Ran Benita  added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42588] Improvements to graphlib.TopologicalSorter.static_order() documentation

[Ran Benita]

New submission from Ran Benita :

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.

--
assignee: docs@python
components: Documentation
messages: 382647
nosy: bluetech, docs@python
priority: normal
severity: normal
status: open
title: Improvements to graphlib.TopologicalSorter.static_order() documentation
versions: Python 3.10, Python 3.9

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com