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

MAINT: Coro is not coro-function. #330

Merged
merged 1 commit into from
Jan 5, 2023
Merged

Conversation

Carreau
Copy link
Member

@Carreau Carreau commented Jan 5, 2023

in the following

async def hello():
    return 1

hello is a coroutine function, and hello() a coroutine. Fix the vocabulary and ensure that the function is properly used.

Copy link
Member

@davidbrochart davidbrochart left a comment

Choose a reason for hiding this comment

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

I think the vocabulary should be coroutine and async function. coroutine-function is confusing.

@Carreau
Copy link
Member Author

Carreau commented Jan 5, 2023

coroutine function is the official term.

See inspect module iscoroutinefunction and iscoroutine.

Async-def function is ambiguous as it could also be an async-generator-function, which when called would be an async-generator

I mean those things have known, defined meaning, I don't see why we should use a different terminology here.

If you really insist we can change, but coroutine is really the instantiated object, so I'd like not to use it.

@davidbrochart
Copy link
Member

Thanks, "coroutine function" seems appropriate then.

@Carreau
Copy link
Member Author

Carreau commented Jan 5, 2023

I'm going to close and reopen to trigger CI.

@Carreau Carreau closed this Jan 5, 2023
@Carreau Carreau reopened this Jan 5, 2023
in the following

```python
async def hello():
    return 1
```

`hello` is a coroutine function, and `hello()` a coroutine.
Fix the vocabulary and ensure that the function is properly used.
Copy link
Contributor

@blink1073 blink1073 left a comment

Choose a reason for hiding this comment

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

Thanks!

@blink1073
Copy link
Contributor

The check release failure is unrelated, I'll fix it separately.

@blink1073 blink1073 merged commit 589bf43 into jupyter:main Jan 5, 2023
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.

3 participants