-
Notifications
You must be signed in to change notification settings - Fork 491
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
cmd/bosun: promote scalars to numberSets #1137
Conversation
Other functions (des, dropge, drople, limit, nv) can also benefit from scalar promotion. But since they are not reduction functions we'll need another way to do the tag matching. They may come in a later PR. At least we get forecastlr and percentile now. |
drop__ could add a fair amount of power I think, something like:
Unless there is another way to do this... |
Yes, they will happen. There's just no obvious common way to do them all now, since they don't have the same function signature. |
43d1c06
to
3e9de8a
Compare
Ok, now does the drop functions. My previous note is somewhat wrong. NV and limit should always remain scalars. Des could use this, but in reality that doesn't really make sense. |
docs need to be updated to reflect the drop funcs |
6281f33
to
8585010
Compare
Updated with dropg, dropl, and docs. |
8585010
to
fa76640
Compare
Will fix #1064 |
@@ -224,3 +224,31 @@ func TestQueryExpr(t *testing.T) { | |||
} | |||
} | |||
} | |||
|
|||
func TestScalarPromotion(t *testing.T) { | |||
tests := map[string]map[string]Value{ |
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 is a really confusing data structure to only hold one test. Either add more or inline the fields (or make an anonymous struct for test cases).
fa76640
to
20bb26b
Compare
Updated with better tests - now combined with the existing expr tests, it just knows about types now. |
cmd/bosun: promote scalars to numberSets
No description provided.