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 Level to Value() #43

Draft
wants to merge 17 commits into
base: main
Choose a base branch
from
Draft

Add Level to Value() #43

wants to merge 17 commits into from

Conversation

krowvin
Copy link
Collaborator

@krowvin krowvin commented Aug 22, 2024

  • Class to allow fetching of levels and levelById - allow endusers to use this themselves to get data
  • Update Value to accept a levelId parameter
  • Update Value to handle levelId as if it were a timeseries Value
  • Create sample form to test various ways you might call a level into your report

Solves

@krowvin krowvin requested a review from DanielTOsborne August 22, 2024 02:51
@krowvin krowvin self-assigned this Aug 22, 2024
@@ -293,7 +293,13 @@ def processDateTime(value, key, extra_part=None):
# If the levelId is set, go for that first
Copy link
Collaborator Author

@krowvin krowvin Aug 22, 2024

Choose a reason for hiding this comment

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

I just had the realization, what happens if a user sets a levelId in the Value() and then tries to query a timeseries?

I think I need to make tests with both present. Because the deep copy will continue with the levelId and would just continue to render of the same level Id I think.

Better way to go about it I think is to check if tsid (or params) or levelId change between calls of the Value()

For example

Value(
levelId="some.level.ID"
)
Value(
tsid="some.tsid.id"
)

And in the code notice that tsid was None before and is now set, then set levelId to None

Copy link
Collaborator

Choose a reason for hiding this comment

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

You'll need to change the logic to detect which was provided by the user and null out the other for this instance.

Honestly I'm not super thrilled with the static backed defaults but that would've been a huge diversion from the existing repgen behavior.

@krowvin
Copy link
Collaborator Author

krowvin commented Aug 26, 2024

Need to test math comparison operators

Level vs Level

Level vs Timeseries

How should a single level be compared against multiple timeseries?

Should it only apply if the self.value is set?

@MikeNeilson
Copy link
Collaborator

Generally I think the same math follows, as already in value, if the LocationLevel is a single value it should be treated as a scalar, if it's a "time series" of values it should be treated like any other time series.

The concept of "Location Level" (or you may want to use "SiteReferenceMetaData" here, that's what we're going to call it in OpenDCS baring someone coming up with a better name) really just determines how it's looked up, changed by others, and when what should be presented, it's still ultimately a scalar value or time series of values.

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.

2 participants