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

Add optional redraw parameter to extendTraces/prependTraces #6682

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

drderiv
Copy link

@drderiv drderiv commented Jul 24, 2023

An optional parameter has been added to extendTraces and prependTraces that specifies whether the graph should be redrawn after a new group of traces has been appended or prepended, as the case may be. This can be useful when the time to redraw cannot keep up with the frequency in which new data arrives, or when data arrives so fast that updating the graph with each trace addition is distracting. Importantly, this change is backwards-compatible with any prior version, as the optional redraw parameter defaults to true, thus, unless explicitly set to false, graphs will continue to be redrawn after each trace addition.

drderiv added 2 commits July 24, 2023 11:44
An optional parameter has been added to extendTraces and prependTraces that specifies whether the graph should be redrawn after a new group of traces has been appended or prepended, as the case may be.  This can be useful when the time to redraw cannot keep up with the frequency in which new data arrives, or when data arrives so fast that updating the graph with each trace addition is distracting.  Importantly, this change is backward compatible with any prior version, as the optional redraw parameter defaults to true, thus, unless explicitly set to false, graphs will continue to be redrawn after each trace addition.
@archmoj archmoj added feature something new community community contribution labels Jul 24, 2023
@alexcjohnson
Copy link
Collaborator

Thanks @drderiv - this is an interesting concept and clearly useful! I'm a little worried about it though, as the user would need to be pretty careful to end any particular chain of operations with redraw=true, otherwise some data would be missing when the graph looks static.

What if we instead added a throttle parameter, that specified a minimum number of milliseconds between redraws but then we could schedule one last update to happen that much time later if no more data arrives? And it would default to zero to match the current behavior.

@drderiv
Copy link
Author

drderiv commented Jul 25, 2023

To clarify your concern, it's that a user does a chain of extendTraces, for example, explicitly setting redrawGraph=false along the way, and then fails to make a call with either no redrawGraph specification or redrawGraph=true and thus the graph does not get updated?

The way I am currently using it is like this: Plotly.extendTraces(graph, data, [0], undefined, counter++ % 10 == 0), with calls to the function being event driven when new data is available. It's true that if data stops flowing for some reason there could be, in my example, up to 9 un-plotted points that may never show up. I do believe your suggested solution would solve that issue and can't think of any negatives associated with that off the top of my head.

drderiv added 5 commits July 31, 2023 12:08
Add redrawMinimumInterval parameter to config for use with extendTraces/prependTraces.  Fully backwards compatible as default is zero.
Revert previous changes and attempt to implement suggestion of @alexcjohnson to use throttle functionality for extendTraces/prependTraces minimum redraw interval.
Update to reflect change in approach suggested by @alexcjohnson.
@gvwilson gvwilson self-assigned this Jul 5, 2024
@gvwilson gvwilson removed their assignment Aug 2, 2024
@gvwilson gvwilson added the P2 considered for next cycle label Aug 9, 2024
@gvwilson
Copy link
Contributor

@alexcjohnson any further thoughts on this one? thanks - @gvwilson

@alexcjohnson
Copy link
Collaborator

Ah, I missed that @drderiv reworked this in line with my suggestion - thank you! This looks great. I haven't tested it, but assuming it works as intended I'm quite happy to accept this.

I suspect adding a programmatic test of this will be tough - and the change is only in the non-default setting anyway, so manual testing may be the best way to handle this one.

@gvwilson gvwilson assigned archmoj and unassigned alexcjohnson Aug 22, 2024
@gvwilson
Copy link
Contributor

thanks @alexcjohnson - much appreciated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community community contribution feature something new P2 considered for next cycle
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants