-
-
Notifications
You must be signed in to change notification settings - Fork 31.8k
graphlib.TopologicalSorter.prepare() should be idempotent #130914
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
Comments
cc @pablogsal |
@tim-one what do you think Tim? |
I like the simple rule That said, I think it would be easy to implement (if not to document) what the OP appears to want: Altermative: leave if self._ready_nodes is None:
self.prepare()
elif self._npassedout:
raise ValueError("static_order() cannot be called after work has started") |
On second thought, |
One consequence of reusing >>> from graphlib import TopologicalSorter
>>> t = TopologicalSorter()
>>> list(t.static_order())
[]
>>> t.prepare()
>>> t.prepare() While a non-empty one doesn't allow that: >>> from graphlib import TopologicalSorter
>>> t = TopologicalSorter({'a': 'bc'})
>>> t.add(*'abc')
>>> list(t.static_order())
['b', 'c', 'a']
>>> t.prepare()
Traceback (most recent call last):
...
ValueError: cannot prepare() after starting sort |
…empotent (python#131317)" This reverts commit c1b42db.
…python#131317) Closes python#130914: Make graphlib.TopologicalSorter.prepare() idempotent Relax the rules so that `.prepare()` can be called multiple times, provided that no work has been passed out by `.get_ready()` yet.
…python#131317) Closes python#130914: Make graphlib.TopologicalSorter.prepare() idempotent Relax the rules so that `.prepare()` can be called multiple times, provided that no work has been passed out by `.get_ready()` yet.
Feature or enhancement
Proposal:
Proposed Behaviour
TopologicalSorter.prepare()
should be idempotent such that calling it repeatedly without draining anything is not an error:If you have called
.get_ready()
/.done()
then callingprepare()
is probably a programming error and this would raise:Rationale
TopologicalSorter.prepare()
raises an exception if you call it twice:This is rather unfortunate because if you return a
TopologicalSorter
that is prepared:because then you cannot run
.static_order()
on it:while if you don't
prepare()
, you then require the caller to do it, meaning the function that populates and returns aTopologicalSorter
didn't leave it in a prepared state. It seems appropriate for such a function to callprepare()
in order to leave the TopologicalSorter ready to iterate and also closed for the addition of new nodes/predecessors.Therefore I think
prepare()
should be idempotent.Has this already been discussed elsewhere?
This is a minor feature, which does not need previous discussion elsewhere
Links to previous discussion of this feature:
This is related to #91301 which discusses removing
TopologicalSorter.prepare()
entirely. Doing so would bypass this issue.Linked PRs
The text was updated successfully, but these errors were encountered: