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

rename TASK_STATUS_IGNORED to TASK_STATUS_STUB #639

Open
belm0 opened this issue Sep 1, 2018 · 9 comments
Open

rename TASK_STATUS_IGNORED to TASK_STATUS_STUB #639

belm0 opened this issue Sep 1, 2018 · 9 comments

Comments

@belm0
Copy link
Member

belm0 commented Sep 1, 2018

The name TASK_STATUS_IGNORED is disconcerting to users of an async API (who may not be aware of the intricacies of start() vs. start_soon()) because it sounds like if they leave that default in place, something significant might get ignored. It's important to find a good name since this constant will appear in virtually all API's using Trio, and we can't rely on every author to document its meaning in every function signature it appears.

TASK_STATUS_STUB would be more descriptive for both API authors and users.

User: : "it's a harmless default which I don't necessarily need to learn about yet"
Author: "it's a dummy object which will ignore calls to started() and custom/future extensions"

@njsmith
Copy link
Member

njsmith commented Sep 1, 2018

Hmm, maybe. or DEFAULT, or PLACEHOLDER...?

@belm0
Copy link
Member Author

belm0 commented Sep 1, 2018

As someone reading a library implementation, if I saw TASK_STATUS_DEFAULT or TASK_STATUS_PLACEHOLDER I would assume they were a integer codes, not an object which sends method calls to /dev/null. Stub is much more descriptive.

As someone reading an API (perhaps knowing that async functions need to be called with await and not knowing of or needing to use nursery.start()), any docile-sounding constant not calling my attention would be fine. The task_status arg is not for end users, it's normally set internally by nursery.start().

@smurfix
Copy link
Contributor

smurfix commented Sep 1, 2018

STUB is perfect.

@njsmith
Copy link
Member

njsmith commented Sep 1, 2018

I'm not ruling it out, but I hesitate because to me STUB makes it sound like something I'm supposed to be replacing with my own implementation...

TASK_STATUS_NOOP? TASK_STATUS_NONE?

Anyone who's implementing an API that takes task_status as an argument will by definition have to have wrapped their head around the whole start framework, so I'm not as worried about confusing them.

We could also consider structuring the name differently, e.g. DEFAULT_TASK_STATUS, NO_TASK_STATUS...

@belm0
Copy link
Member Author

belm0 commented Sep 1, 2018

TASK_STATUS_NOOP? TASK_STATUS_NONE?
DEFAULT_TASK_STATUS, NO_TASK_STATUS

these all sound like enumerated codes, not a mutable object to communicate task status

@smurfix
Copy link
Contributor

smurfix commented Sep 1, 2018

maybe TASK_STATUS_SHIM or NOOP_TASK_STATUS

@njsmith
Copy link
Member

njsmith commented Sep 1, 2018

these all sound like enumerated codes, not a mutable object to communicate task status

Is that so bad? In your original post it sounded like your main concern was people reading the docs for APIs that use this, and reassuring them that they don't really need to understand this right now.

@belm0
Copy link
Member Author

belm0 commented Sep 2, 2018

In your original post it sounded like your main concern was people reading the docs for APIs that use this, and reassuring them that they don't really need to understand this right now.

Yes, it prompted wanting a rename and is the most important thing to rectify. However as an author of API's using Trio, I selfishly would like to make the name more descriptive at the same time. Having just come to Trio with new eyes, the names that threw me off the most were this and ParkingLot. These two have a higher degree of being misleading or non-descriptive than say, nursery, which has received a lot of attention.

The Trio API on the whole has far better naming than most projects (nursery and start / start_soon are great examples), sincere kudos! There are only a couple of renames I can suggest that I believe are a decent value.

@belm0
Copy link
Member Author

belm0 commented Sep 3, 2018

Part of the issue stems from the argument name task_status itself, though I can't suggest something more descriptive that isn't verbose.

  • task_status=TASK_STATUS_IGNORED - task_status sounds like an input to the function, not an output. So when combined with TASK_STATUS_IGNORED it's worrisome to API users because it sounds like something they need to know about and set correctly.

  • task_status=TASK_STATUS_STUB - "stub" provides a good hint that task_status is not an input constant and there is some sane default being provided. For API authors it's more descriptive as well.

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

No branches or pull requests

3 participants