-
Notifications
You must be signed in to change notification settings - Fork 225
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
Doc tweaks for ev/deadline and ev/with-deadline #1410
Doc tweaks for ev/deadline and ev/with-deadline #1410
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
It is okay, but it doesn't explain the relationship between If I didn't read the background discussion, I would say what is this? |
@amano-kenji, I would like to ask you to edit your last comment to remove vulgarism, please. |
The only relationship between |
AFAIU, there isn't necessarily a relationship between IIUC, establishing one is entirely up to the programmer. There doesn't need to be one for this function operate. I've started to take some notes for potential additions / changes to the website docs here. The kind of thing you are describing might be a good addition there. |
If I was the author of this pull request, I would add more background information to the docstring for I would at least document how cancelling They will likely think
That's not good enough. This will lead to confusion or reverse-engineering. Perhaps, people can actually freeze If the background information is too verbose, then |
@amano-kenji wrote:
The problem would be that that isn't necessarily true. One can contrive ways in which this would not be the case. Say you have a task (A) and a task (B). B knows about A and knows A has an interest in the cancellations of fibers it runs under a deadline.
Documenting what is a Fiber, and a Task, and a Root Fiber in every single function using them is not a good idea. That's way too verbose. It shouldn't be surprising or unexpected for one to glean those terms from referencing the language documents. Edit: |
I would likely mention something like
This is not too verbose. You can probably condense it down to
After reading this, people will realize cancelling the root fiber leads to cancellation of the current fiber. |
If anything other than the root fiber of |
You can freeze |
It should be possible. The likelihood that someone will without purposefully mucking around in Basically, it counts on the fact that Any fiber that doesn't do one of those things - yield, or yield to the event loop - is in danger of freezing Janet in general, not just |
I read the definition of (defmacro ev/with-deadline
`Run a body of code with a deadline, such that if the code does not complete before
the deadline is up, it will be canceled.`
[deadline & body]
(with-syms [f]
~(let [,f (coro ,;body)]
(,ev/deadline ,deadline nil ,f)
(,resume ,f)))) I wrote test.janet (def fiber (coro
(try
(forever (ev/sleep 1))
([err f]
(eprint "fiber cancelled")
(propagate err f)))))
(def task (ev/spawn
(try
(forever
(ev/sleep 1))
# ignore errors
([err f]))))
(pp (ev/deadline 5 task fiber))
(print "ev/deadline returned...")
(resume fiber) Running this script results in
Thus, Actually, |
Borrowing from my review: This also shows the body is still alive... (pp
(protect
(ev/with-deadline 0.01
(protect (ev/sleep 2)) # the call to `ev/sleep` receives the error...
(pp (fiber/status (fiber/current)))
(pp [:fib (fiber/current)])
(pp [:lineage (debug/lineage (fiber/root))])
(flush)
(yield :hi)))) With results: :alive
(:fib <fiber 0x06EB603B8050>)
(:lineage @[<fiber 0x06EB6026CED0> <fiber 0x06EB603B7BF0> <fiber 0x06EB603B7FE0> <fiber 0x06EB603B8050>])
(true :hi)
nil And (let [fib (coro
(pp [:fib (fiber/current)])
(pp [:lineage (debug/lineage (fiber/root))])
(ev/sleep 0.2)
(yield :hi))]
(pp [:tocancel (ev/deadline 0.1 nil fib)])
(pp (protect (ev/sleep 5)))
(print "Cancellation has occurred.")
(resume fib)) Has results:
|
@amano-kenji wrote:
It already mentions in both the new documentation and the old that it is The current documentation for |
I think |
The current docstring for
deadline is a terrible choice of word for Deadline for
|
That's going to argue what words mean or imply. I'm not sure if there is a better selection of a name. In life, one often continues and completes work after a deadline has failed to be met, but it means they did the work for nothing (it's unaccepted as past due). This is exactly what happens to |
That would be easier to understand.
|
A better docstring would be
|
The proposed docstring (from C side) is as below:
Your proposal for a better one fails to outline what happens to |
This wording is still confusing. This made me think I'd rather write
I may even add
Deadline for something means death for the thing in plain english. The documentation should make it clear that death directly comes to |
That would be wrong. The thing checked for liveliness beyond a deadline expiration is the thing which has a deadline set upon it.
Adding an ambiguous truth is likely less useful than adding nothing and letting it remain true that "there is nothing documented about anything being done to |
Then, this is better.
|
Wait is untrue, there is no waiting, according to what you said earlier, |
How about this?
Deadline for
How do I know whether the docstring is or is not incomplete or esoteric? I want to eliminate the possibility of connotations. Expecting users to take one interpretation over another is bad. |
You're confusion probably stems from a misunderstanding of the term deadline. A deadline means the work done is no longer timely, not that it isn't done, or isn't continuing to be worked upon. Your proposed doc string leaves out key details such as the default values, the necessary type for Finally, the second thing in your bullet list goes with the idea that people read documentation and make assumptions where the documentation outlines nothing occurring. If a behavior is not documented, it either doesn't happen, or should be documented. In this case, the documentation would align with "it doesn't happen, nothing to document". |
It is not a misunderstanding. Deadline by itself doesn't explicitly mention which things die after deadline. In matters of death, we should explicitly mention which things are killed directly. Oops, I forgot to mention |
A deadline implies the death of validity of work, not the death of that doing the work. |
A deadline has many meanings. It's not just one thing. It can mean a different thing to a different person. One of definitions on dictionary.com says
This is the definition I thought of. Deadline for prisoner A means death for prisoner A. Unless you want to define the meaning of deadline in |
That's confusing as hell....... |
I'm still working on These are very much in draft state, but FWIW. I'm very much against inventing fictions for the most part. |
@amano-kenji Please stop with the use of profanity and consider rephrasing the instances made already. It is completely unnecessary. Alternative expressions would do fine. |
Fine... But, that docstring doesn't explain the intention behind |
Which docstring are you referring to? The current one? I don't necessarily disagree. |
@amano-kenji @sogaiu |
This sounds esoteric and confusing. It would throw me off balance if I haven't been in the discussion for the last few days. I would write
If the body of |
I didn't test, but IIUC, the website docs say this about the event loop:
Have you read that page recently? |
So, reception of the cancellation error will be delayed after deadline if the body of |
As I mentioned, I have not tested. |
That's closer to the wording @sogaiu has in what he linked which I'm fine with.
It does exactly that, it blocks the whole task... It's why we can't say that |
This has a disorienting effect on my psyche. This is a riddle unless the relationship between the root fiber and the body of The word, root fiber, still throws me off balance. The value of I kind of understand the root fiber, but I don't really. |
That relationship I believe is beyond the scope of a docstring (wherein it'd likely pertain to documenting the event loop in more detail than belongs in a docstring). That said, something similar to your wording of
is fine with me. @sogaiu has a link that if you look has similar wording for one of the ideas of what the docstring should be. |
Re:
I think this is not a reasonable characterization for a few reasons:
If you are thrown off-balance, perhaps you should consider whether there are other factors that are contributing to that as well. Are you calm right now? Or are you caught up in emotion? |
I read the event loop documentation three times from top to bottom while working on j3blocks. Perhaps, the documentation doesn't inculcate the concept of root fiber enough. Root fiber just slips off my memory... |
Glad you read them. I am thinking to suggest the addition of a glossary to the website docs. You can see the beginning of a draft here. |
I think https://janet-lang.org/docs/event_loop.html doesn't account for the fact that humans don't remember concepts they pass by just once. Only after they encounter example making you think about root fiber several times, root fiber sinks in your psyche. |
Your observations are not lost on me. It's part of why I work on docstrings and examples and have been doing so over the past few years. |
Because humans just forget words they pass by briefly, docstring should reinforce the concept of root fiber... here and there....
This reinforces the concept of root fiber and task, and makes people read the documentation again... |
I don't agree with that necessarily though. Update: the text that the above was a response to changed after I wrote the text above. So I am adding a fuller response below. I find the reasoning in:
to be flawed. However, I don't disagree that occasional hints / reinforcement in docstrings is necessarily a bad idea and I have actually been doing this. Regarding "humans just forget words they pass by briefly" -- knowing this, I think the authors of documents can make an effort, but at the same time, readers can also take steps to help themselves in ways customized to their own needs. One concrete step one can take is to take one's own notes along with writing and keeping sample code. This can also include the projects one has worked on. This method has worked well for me -- I am often able to recreate enough context / knowledge by looking over what I produced earlier. It's also much safer from other parties modifying it (^^; I'll stop here though because this is not about this issue directly. |
I'll remind you that bakpakin has stated that this is an issue tracker and not a chat room. Please consider our discussion from yesterday regarding drafting your responses and going over them / revising them before posting. |
b49c609
to
d549df5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
d549df5
to
592ac49
Compare
This is an attempt to address #1349.
Background can be seen in that issue, and a summary is available at this comment.
Thanks to @amano-kenji and @llmII for discussions and investigations.