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

Support Receives section for generator.send(...) params #145

Merged
merged 5 commits into from Apr 9, 2019

Conversation

jnothman
Copy link
Member

@jnothman jnothman commented Nov 14, 2017

fixes #47

@jnothman jnothman changed the title Support Sent section Support Sent section for generator.send(...) params Nov 22, 2017
@jnothman
Copy link
Member Author

Ping @Dreem-Devices, @tacaswell, @stefanv who commented on the original issue.

doc/format.rst Outdated
@@ -250,13 +250,21 @@ The sections of a function's docstring are:
Support for the **Yields** section was added in `numpydoc
<https://github.com/numpy/numpydoc>`_ version 0.6.

7. **Other Parameters**
7. **Sent**
Copy link
Contributor

Choose a reason for hiding this comment

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

I am a bit torn about the order of Yields and Sent. Putting Sent first feels more natural to the way you use send (in that the next value to come out probably depends on the value you sent in or you would not be bothering to send anything in!).

On the other hand it yields a non-trivial value before you can send in a non trivial value so it should stay in this order.

On the other other hand, you do send in None first to prime the generator (but that is not what is being documented in this block).

Copy link
Member Author

@jnothman jnothman Nov 29, 2017

Choose a reason for hiding this comment

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

I had thought the idiom was to use next initially, then .send later.

I think the discomfort is partially with the sense of the word "Sent", which connotes a departing aspect (perhaps because it is not agentive). "Receives", in contrast, seems to fit better after "Yields".

Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting, I had not seen that idiom. I like sending None as it special-cases the first round less, but that is neither here nor there for this discussion.

"The generator is sent ..." vs "The generator receives ..."

The first is odd because we drop the 'is' in the section title and the second is odd because it feel more passive.

Copy link
Contributor

Choose a reason for hiding this comment

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

"receives" sounds more correct than "sent" in my head.

Choose a reason for hiding this comment

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

I prefer "receives" as well. Another possibility I can think of is "accepts," but I don't think I like that as much as "receives".

@tacaswell
Copy link
Contributor

I'm going to ping my co worker @danielballan who is much better at naming things than I am.

@jnothman jnothman changed the title Support Sent section for generator.send(...) params Support Receives section for generator.send(...) params Jan 8, 2018
@rgommers
Copy link
Member

LGTM. Needs a rebase.

@rgommers
Copy link
Member

rgommers commented Apr 7, 2019

@jnothman want to rebase this? I think it can go into 0.9.0

@rgommers rgommers added this to the v0.9.0 milestone Apr 7, 2019
@@ -216,6 +235,38 @@ def test_yields():
assert desc[0].endswith(end)


def test_sent():
section = doc_sent['Receives']
assert_equal(len(section), 2)
Copy link
Member

Choose a reason for hiding this comment

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

CI isn't happy: NameError: global name 'assert_equal' is not defined

You can just change this to plain assert

Copy link
Member

Choose a reason for hiding this comment

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

and some more of the same several lines below

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

support for coroutine
6 participants