-
-
Notifications
You must be signed in to change notification settings - Fork 404
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
Simplified operation modes supported by Dynamicmap #1238
Conversation
@philippjfr I now think this PR should be about cleaning things up only. Once we have removed everything we want to, I think we should merge and I'll follow up with a PR to add things back (updating docs, adding tests, adding more validation etc). I will delete the bits in the DynamicMap tutorial that are out of date (it might no longer be a coherent story but that is what I'll fix in the next PR). Sound reasonable? |
I've now removed the sections of the DynamicMap tutorial that no longer apply - everything still in the notebook works as before. In the next PR I'll rewrite this tutorial to try and cover the same sort of examples but using streams. We might also want to decide on how to split between a DynamicMap tutorial and a streams tutorial. @philippjfr I think this PR is ready for review - is there anything else that can be cleaned up and removed before I work on reintroducing things? |
I always like to see lots of code disappear. 700 lines gone! |
Looks great to me! |
Feel free to click 'Merge' then! :-) |
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
This PR greatly simplified DynamicMap, removing all concept of 'modes' except for 'sampled' mode: 'open', 'bounded', 'generator' and 'counter' mode are all gone (implicitly DynamicMaps are now always 'bounded').
In addition, keys cannot be returned by the callback.
Action items:
Edit:
The following items will be assigned to a follow up PR after this one is merged.
I might recommend leaving the last point (updating the tutorials) for another PR - the earlier this is merged, the better tested it will be before release and the bigger chance we can hear any objections from our users. We don't think anyone has ever really other used anything other than 'bounded' mode with callables (possibly with streams) but we can't be certain.