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

Added event method to DynamicMap #935

Merged
merged 1 commit into from Oct 17, 2016

Conversation

Projects
None yet
2 participants
@jlstevens
Member

jlstevens commented Oct 16, 2016

When prototyping the streams system, one thing we were wondering about was how to easily access streams from a DynamicMap in order to call update. The problem is you either need to keep a handle on the stream object or would need to use some mechanism to access the stream via some (optional) assigned name or by index. This poses some difficult problems and this PR suggests an alternative approach that bypasses this issue completely.

The event method on a DynamicMap does not deal with stream objects, just stream parameters. The appropriate streams that require updating are computed behind the scenes which works well as the identity of the streams is often of less interest to the user than the set of stream parameters (these are the keyword arguments passed to the DynamicMap callback).

If nothing else, the event method is is useful for debugging it will always allow print statements to appear in the notebook. Note that this isn't true for direct Bokeh interaction streams which sends such print statements to the web console:

image

Earlier on, I proposed that the update method of streams could also be renamed to event for consistency - unfortunately update already exists on DynamicMap as it inherits the method from HoloMap otherwise I would have called this method update too.

I no longer feel this is particularly important. This PR will mean that users rarely use an update method in any context: it is rarely used by users on HoloMaps or DynamicMaps and this new event method means users won't have to use the update method on streams either. That said, I'm still open to renaming update on streams to event if we feel this is better for overall consistency.

Lastly, I will want to have a tutorial demonstrating the event method as a way of building visualizations with streams. In particular, I will want a notebook demonstrating it with a visualization using a mix of kdims and dimensioned streams.

@philippjfr

This comment has been minimized.

Member

philippjfr commented Oct 17, 2016

Looks good and it's what we discussed. Ready to merge?

@jlstevens

This comment has been minimized.

Member

jlstevens commented Oct 17, 2016

If you are happy leaving streams with an update as opposed to event method, then I think it is ready. As the streams API is new, we can still do whatever renaming we want.

@philippjfr

This comment has been minimized.

Member

philippjfr commented Oct 17, 2016

Yes, I'm happy with that. Merging.

@philippjfr philippjfr merged commit 42dd118 into master Oct 17, 2016

3 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.04%) to 72.958%
Details

@philippjfr philippjfr deleted the dmap_event branch Jan 7, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment