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.

Sign up for GitHub

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

Jump to bottom

clarified Nursery.start vs Nursery.start_soon #972

Closed
wants to merge 8 commits into from
Closed

clarified Nursery.start vs Nursery.start_soon #972

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
975f55c
clarified documentation around Nursery.start vs start_soon
rotu Mar 7, 2019
82807c7
Update history.rst
rotu Mar 7, 2019
1d4c098
Revert "Update history.rst"
rotu Mar 7, 2019
c235dee
Rewrote changes with some VERY helpful feedback from oremanj
rotu Mar 9, 2019
d5ae8ec
reworded what the code before `task_status.started()` is for in examp…
rotu Mar 9, 2019
7ddfddd
reverted stray `
rotu Mar 9, 2019
d4cd696
Polish locution
rotu Mar 9, 2019
57cd417
Removed extraneous "all"
rotu Mar 10, 2019
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
24 changes: 12 additions & 12 deletions docs/source/reference-core.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -834,13 +834,13 @@ Nursery objects provide the following interface:

Note that this is *not* an async function and you don't use await
when calling it. It sets up the new task, but then returns
immediately, *before* it has a chance to run. The new task won’t
actually get a chance to do anything until some later point when
you execute a checkpoint and the scheduler decides to run it.
If you want to run a function and immediately wait for its result,
then you don't need a nursery; just use ``await async_fn(*args)``.
If you want to wait for the task to initialize itself before
continuing, see :meth:`start()`.
immediately, *before* it has a chance to run. At some future
time, (which might be closing of the nursery context, but no
later), all tasks requested with :meth:`start_soon` will begin,
Copy link
Member

Choose a reason for hiding this comment

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

This still could be read to imply that all of the tasks will be started at once, which isn't a guarantee we want to make. What if anything do you think is unclear about taking the previous wording ("won't actually get a chance to do anything ... the scheduler decides to run it") and adding a sentence like "Tasks might start running in a different order than the calls to start_soon()"?

Copy link
Contributor Author

@rotu rotu Mar 9, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

You're right - it does imply a single time. Rephrased to "Tasks requested with :meth:start_soon will begin, at no particular time, in no particular order (but no later than the closing of the nursery context)."
I wanted to avoid mention of the scheduler here. The important thing is that ordering and timing get arbitrary when you call start_soon - I don't think the reader will get that unless they have a good grasp of what the scheduler does and does not promise.

in no particular order. If you want to run a function to
completion, then you don't need a nursery; just use
Copy link
Member

Choose a reason for hiding this comment

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

The function always runs "to completion" (as opposed to being abandoned in the middle somewhere) regardless of whether you use await or nursery.start_soon. What are you trying to communicate here that you didn't feel was adequately communicated with "and immediately wait for its result"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The start function runs a function and immediately waits for a result too. It just so happens that the result might be returned by an intermediate result from a .started call in the middle.. I wanted to convey the difference. How about "If you want to run a function and block until it finishes" ?

``await async_fn(*args)``. If you want to run some part of the
Copy link
Member

Choose a reason for hiding this comment

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

maybe "If you want to wait for the child task to finish starting up before the parent is allowed to proceed"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, because "finish starting up" is not a meaningful concept until you've already bought into the abstraction of Nursery.start(). A task scheduled with start_soon might never call a task_status.started() function - but any programmer would say that task starts and ends and infers that it "finishes starting" somewhere in between. There needs to be some concept of the child signaling to the parent, even if we don't use the word "signal".

child task before the parent task proceeds, see :meth:`start()`.

It's possible to pass a nursery object into another task, which
allows that task to start new child tasks in the first task's
Expand All @@ -867,9 +867,9 @@ Nursery objects provide the following interface:
.. method:: start(async_fn, *args, name=None)
:async:

Like :meth:`start_soon`, but blocks until the new task has
finished initializing itself, and optionally returns some
information from it.
Like :meth:`start_soon`, but blocks until the new child task
signals that it may unblock, optionally returning additional
Copy link
Member

Choose a reason for hiding this comment

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

"signals" is a slightly overloaded word to use here due to Unix signals. Maybe "until the new child task indicates that it's finished starting up"? And I think the original "and optionally returns some information" is better than "optionally returning additional information" -- the more active phrasing tends to communicate more clearly IMO.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changing to "indicates"

information.

The ``async_fn`` must accept a ``task_status`` keyword argument,
and it must make sure that it (or someone) eventually calls
Expand All @@ -878,9 +878,9 @@ Nursery objects provide the following interface:
The conventional way to define ``async_fn`` is like::

async def async_fn(arg1, arg2, *, task_status=trio.TASK_STATUS_IGNORED):
...
... # some work the parent might wait for
task_status.started()
...
... # some work that may run in parallel with the parent

:attr:`trio.TASK_STATUS_IGNORED` is a special global object with
a do-nothing ``started`` method. This way your function supports
Expand Down
1 change: 1 addition & 0 deletions newsfragments/970.doc.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Clarified documentation around Nursery.start_soon and Nursery.start, that start_soon does not guarantee task ordering, and when you should use each.
Copy link
Member

Choose a reason for hiding this comment

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

@njsmith - do we usually add newsfragments for documentation updates?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@oremanj @njsmith Any opinions? I don't care whether we leave it in or take it out, but I'd like this pull request resolved