Skip to content
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
Merged

Fixed PlotReset stream parameter #2665

merged 1 commit into from May 8, 2018

Conversation

@philippjfr
Copy link
Member

@philippjfr 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.

@jlstevens
Copy link
Contributor

@jlstevens jlstevens commented May 8, 2018

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

@philippjfr
Copy link
Member Author

@philippjfr philippjfr commented May 8, 2018

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

@jlstevens
Copy link
Contributor

@jlstevens 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
Copy link
Member Author

@philippjfr 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
Copy link
Contributor

@jlstevens 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
Copy link
Contributor

@jlstevens 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
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
@philippjfr
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
Labels
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants