-
Notifications
You must be signed in to change notification settings - Fork 77
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
ENH: ArchiveTimePlot Fetch Data on X-Axis Change #1172
base: master
Are you sure you want to change the base?
Conversation
As I'm reading back through my code, I'm wondering if this should be an optional change. Should add a parameter to the ArchiveTimePlot constructor for getting data like this. |
Made the changes optional so that it can be enabled/disabled as required. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me. I appreciate you breaking up the content in updateXAxis into smaller methods, it makes it easier to read :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you very much for the clean up! And the functionality is working well.
I think my only main concern is that it's possible to set up a plot without caching in such a way that it would call archiver every second if the PV is updating at 1Hz. (A short timespan would cause the x axis to move on every update). Defaulting cache_data to true is a good idea, but maybe we can still guard against that possibility.
You're right, this can be an issue. I was told not to worry about it because our archiver appliance can handle it, but I shouldn't assume that's true of everyone's setup. I'm trying to come up with some way of reducing those calls. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice!
This change prompts the ArchiveTimePlot to get data from the Archiver Appliance every time the x-axis' range changes.
Previously, archive data was only fetched when the user showed a section of the plot that had no data. With this set up, zooming out will fetch a optimized data (averaged w/ min/max error bars), but zooming back in would still optimized data. This results in the "resolution" looking wrong.
Now zooming in on a section will get the data for the time range shown on the plot. This will result in showing the correct "resolution" at all times.