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

Fixed PlotReset stream parameter #2665

Merged
merged 1 commit into from May 8, 2018

Conversation

Projects
None yet
2 participants
@philippjfr
Member

philippjfr commented May 8, 2018

The reset parameter on the PlotReset stream was shadowing the reset method on Streams, causing errors when the parameter was reset to None on transient events.

@philippjfr philippjfr added the API label May 8, 2018

@jlstevens

This comment has been minimized.

Member

jlstevens commented May 8, 2018

Some other ideas for the new name: plotreset or reset_event?

@philippjfr

This comment has been minimized.

Member

philippjfr commented May 8, 2018

Out of the three I much prefer resetting, it implies an action/event and is fairly short.

@jlstevens

This comment has been minimized.

Member

jlstevens commented May 8, 2018

I'm just brainstorming to see if we can come up with something better. As it is a boolean, I'm thinking isreset or hasreset (not great names, I know).

My main issue with resetting is that it sounds like it is in the process of resetting which I don't think is strictly true: the plot has finished resetting and this boolean is informing the user code of this fact.

If we can't find something better, I agree we can stick with resetting though.

@philippjfr

This comment has been minimized.

Member

philippjfr commented May 8, 2018

My main issue with resetting is that it sounds like it is in the process of resetting which I don't think is strictly true: the plot has finished resetting and this boolean is informing the user code of this fact.

I'd argue whatever action you want to trigger on reset becomes part of the reset so it is in fact still resetting at that point.

@jlstevens

This comment has been minimized.

Member

jlstevens commented May 8, 2018

I'd argue whatever action you want to trigger on reset becomes part of the reset so it is in fact resetting

I thought you might say that. It depends on whether you are thinking of it from the bokeh perspective, or from the perspective of the overall visualization. Anyway, I guess resetting is fine.

@jlstevens

This comment has been minimized.

Member

jlstevens commented May 8, 2018

Would be good to add an example to the streams reference gallery. That can be done is a separate PR though, merging.

@jlstevens jlstevens merged commit 5f2fb06 into master May 8, 2018

4 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 remained the same at 83.316%
Details
s3-reference-data-cache Test data is cached.
Details

@philippjfr philippjfr deleted the reset_stream_fix branch Jul 4, 2018

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