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

ENH: Adds Labels to Timeplot Crosshair #1173

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

Conversation

YektaY
Copy link
Collaborator

@YektaY YektaY commented Feb 25, 2025

Description

Creates labels on the vertical crosshair at the intersection point of the curves. This PR also addresses some bugs in crosshair functionality

Motivation and Context

Screenshots

Screenshot 2025-02-25 at 10 48 00 AM

@YektaY YektaY changed the title ENH: Adds Labels along with a couple fixes to the timeplot crosshair ENH: Adds Labels to Timeplot Crosshair Feb 25, 2025
@YektaY YektaY linked an issue Feb 25, 2025 that may be closed by this pull request
Added in displying channel severity on the crosshair labels.
Added unit tests.
@YektaY YektaY marked this pull request as ready for review March 8, 2025 00:23
Copy link
Collaborator

@jbellister-slac jbellister-slac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking good!

One basic question I apologize for not thinking of sooner, is there a reason for putting all the label code in timeplot.py? It seems like it could be useful to have that in baseplot as well so that other plot types could use it too.

@YektaY
Copy link
Collaborator Author

YektaY commented Mar 12, 2025

This is looking good!

One basic question I apologize for not thinking of sooner, is there a reason for putting all the label code in timeplot.py? It seems like it could be useful to have that in baseplot as well so that other plot types could use it too.

Oh that is a good point, I was mainly focused on it working for the archive viewer. I would be happy to move it into baseplot

@@ -115,6 +116,9 @@ def __init__(self, channel_address=None, plot_by_timestamps=True, plot_style="Li
self.channel = None
self.units = ""

self.severity_raw = -1
self.severity = "N/A"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor optional change,
but we could add the following constants to this file (but not as members of class):

DEFAULT_SEVERITY_RAW = -1
DEFAULT_SEVERITY_STRING = "N/A"

and then import into tests, similar to "MINIMUM_BUFFER_SIZE",
and use for the testing comparisons

could also do this with "No data!" string too.
makes it easier to manage these hard coded values.

-------
None
"""
if not np.isfinite(x_val):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we want to be setting "No data!" here?

"If x_val is not finite ... the label text is set to "No data!"

@nstelter-slac
Copy link
Collaborator

also nice patch!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crosshair coordinates don't follow plot coordinates
3 participants