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

gh-130914: Make graphlib.TopologicalSorter.prepare() idempotent #131317

Merged
merged 6 commits into from
Mar 18, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions Doc/library/graphlib.rst
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,14 @@
function, the graph cannot be modified, and therefore no more nodes can be
added using :meth:`~TopologicalSorter.add`.

A :exc:`ValueError` will be raised if the sort has been started by
:meth:`~.static_order` or :meth:`~.get_ready`.

.. versionchanged:: next

``prepare()`` can now be called more than once as long as the sort has
not started. Previously this raised :exc:`ValueError`.

.. method:: is_active()

Returns ``True`` if more progress can be made and ``False`` otherwise.
Expand Down
9 changes: 9 additions & 0 deletions Doc/whatsnew/3.14.rst
Original file line number Diff line number Diff line change
Expand Up @@ -607,6 +607,15 @@ getopt
* Add support for returning intermixed options and non-option arguments in order.
(Contributed by Serhiy Storchaka in :gh:`126390`.)


graphlib
--------

* Allow :meth:`graphlib.TopologicalSorter.prepare` to be called more than once
as long as sorting has not started.
(Contributed by Daniel Pope in :gh:`130914`)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
(Contributed by Daniel Pope in :gh:`130914`)
(Contributed by Daniel Pope in :gh:`130914`.)



http
----

Expand Down
14 changes: 9 additions & 5 deletions Lib/graphlib.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,13 +90,17 @@ def prepare(self):
still be used to obtain as many nodes as possible until cycles block more
progress. After a call to this function, the graph cannot be modified and
therefore no more nodes can be added using "add".

Raise ValueError if nodes have already been passed out of the sorter.

"""
if self._ready_nodes is not None:
raise ValueError("cannot prepare() more than once")
if self._npassedout > 0:
raise ValueError("cannot prepare() after starting sort")

self._ready_nodes = [
i.node for i in self._node2info.values() if i.npredecessors == 0
]
if self._ready_nodes is None:
self._ready_nodes = [
i.node for i in self._node2info.values() if i.npredecessors == 0
]
# ready_nodes is set before we look for cycles on purpose:
# if the user wants to catch the CycleError, that's fine,
# they can continue using the instance to grab as many
Expand Down
14 changes: 13 additions & 1 deletion Lib/test/test_graphlib.py
Original file line number Diff line number Diff line change
Expand Up @@ -140,9 +140,21 @@ def test_calls_before_prepare(self):
def test_prepare_multiple_times(self):
ts = graphlib.TopologicalSorter()
ts.prepare()
with self.assertRaisesRegex(ValueError, r"cannot prepare\(\) more than once"):
ts.prepare()

def test_prepare_after_pass_out(self):
ts = graphlib.TopologicalSorter({'a': 'bc'})
ts.prepare()
self.assertEqual(set(ts.get_ready()), {'b', 'c'})
with self.assertRaisesRegex(ValueError, r"cannot prepare\(\) after starting sort"):
ts.prepare()

def test_prepare_cycleerror_each_time(self):
ts = graphlib.TopologicalSorter({'a': 'b', 'b': 'a'})
for attempt in range(1, 4):
with self.assertRaises(graphlib.CycleError, msg=f"{attempt=}"):
ts.prepare()
Comment on lines +154 to +156
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
for attempt in range(1, 4):
with self.assertRaises(graphlib.CycleError, msg=f"{attempt=}"):
ts.prepare()
for attempt in range(1, 4):
with self.subTest(attempt=attempt):
self.assertRaises(graphlib.CycleError, ts.prepare)

You can also use subTest() if you want.


def test_invalid_nodes_in_done(self):
ts = graphlib.TopologicalSorter()
ts.add(1, 2, 3, 4)
Expand Down
1 change: 1 addition & 0 deletions Misc/ACKS
Original file line number Diff line number Diff line change
Expand Up @@ -1483,6 +1483,7 @@ Michael Pomraning
Martin Pool
Iustin Pop
Claudiu Popa
Daniel Pope
Nick Pope
John Popplewell
Matheus Vieira Portela
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Allow :meth:`graphlib.TopologicalSorter.prepare` to be called more than once
as long as sorting has not started.
Patch by Daniel Pope.
Loading