-
Notifications
You must be signed in to change notification settings - Fork 150
Adding messages #111
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
Adding messages #111
Conversation
Codecov Report
@@ Coverage Diff @@
## master #111 +/- ##
==========================================
+ Coverage 93.68% 93.74% +0.05%
==========================================
Files 12 12
Lines 1377 1390 +13
==========================================
+ Hits 1290 1303 +13
Misses 87 87
Continue to review full report at Codecov.
|
Do other systems do this? If so then what are their semantics? The alternative here would be to ask the user to implement this sort of logic within their operations, either in their |
clear_msg = ClearMSG() | ||
ignore_msg = IgnoreMSG() | ||
|
||
sout = s.accumulate(acc1) |
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.
What would happen if you had a map before the accumulate? Does map (or all other stream operations) also need to know what to do with ClearMSGs?
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.
I would imagine that all update
methods will need a cutout for messages.
I'm not certain that we can ask the user to modify everything, as there are some cases where we buffer information without user input. For example |
I will have to get back to you about this, sorry busy. If you want to prevent this behaviour from happening, you must insert a I don't like this method but I do like the splitting of Initial thoughts? This ties in with #86 as well I believe. |
(Just for extra reference, @CJ-Wright has already implemented something similar on the |
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.
We may want messages to have a call method which takes in the node? This way we could do things like timing execution.
logger = logging.getLogger(__name__) | ||
|
||
|
||
class ClearMSG: |
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.
You may want a MSG base class so you can just check if an item is an instance if that class
superceded by #249 and its discussion, I think |
Should we allow messages to pass? I think something like this would be useful.
For example here, we send a clear message which should clear various elements of the stream that hold state. Right now, only this is performed:
but I'd find this useful. One could change these messages to node specific, like "CLEAR_ACC", or some control stream with flags like:
Clear(clear_acc=True,...)
.any thoughts? This is meant to open discussion. I find it more convenient than an issue in this case.
The various messages or message handler can perhaps also be handled by dispatching, to allow for more convenient subclassing.
This was a quick thought. If it's of interest in the base class we can discuss adding more.