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

Transformations: Add unary operations to Add field from calculation #75946

Merged
merged 6 commits into from
Oct 6, 2023

Conversation

mdvictor
Copy link
Contributor

@mdvictor mdvictor commented Oct 4, 2023

Screen.Recording.2023-10-04.at.13.33.00.mov

What is this feature?

Add unary operations to Add field from calculation transform, e.g: abs(), exp(), floor(), ceil()

Why do we need this feature?

Enhances functionality of the transformation

Who is this feature for?

Everyone

Which issue(s) does this PR fix?:

Fixes #74489

Special notes for your reviewer:

Please check that:

  • It works as expected from a user's perspective.
  • If this is a pre-GA feature, it is behind a feature toggle.
  • The docs are updated, and if this is a notable improvement, it's added to our What's New doc.

Copy link
Contributor

@oscarkilhed oscarkilhed left a comment

Choose a reason for hiding this comment

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

Nice! Was thinking the other day we should add this! Works well, but left some comments to make the code a bit nicer.

Comment on lines 410 to 431
<div className="gf-form-inline">
<div className="gf-form">
<div className="gf-form-label width-8">Operation</div>
</div>
<div className="gf-form">
<Select
className="width-8 gf-form-spacing"
options={ops}
value={options.operator ?? ops[0].value}
onChange={this.onUnaryOperationChanged}
/>
<div className="gf-form-label width-1">(</div>
<Select
placeholder="Field"
className="min-width-7 gf-form-spacing"
options={fieldName}
value={options?.fieldName}
onChange={this.onUnaryValueChanged}
/>
<div className="gf-form-label width-1">)</div>
</div>
</div>
Copy link
Contributor

Choose a reason for hiding this comment

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

I would prefer if this was using the new form elements. Not sure how consistent it would look with the form for the binary operators, maybe it's not that big of a deal? Maybe it's time to change the binary form the the new form elements as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree! I'll look into that!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Modified the editor to now use new ui elements!

const { unary } = this.props.options;
this.updateUnaryOptions({
...unary!,
operator: v.value! as UnaryOperationID,
Copy link
Contributor

Choose a reason for hiding this comment

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

We can avoid this as by using SelectableValue<UnaryOperationId>, adding a unaryOperationID property to the UnaryOperationInfo and using that property when we are constructing the options for the select.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you! Will modify!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Modified this for both unary and binary editors!

Copy link
Collaborator

@imatwawana imatwawana left a comment

Choose a reason for hiding this comment

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

Made a couple small suggestions.

Comment on lines 111 to 112
- **Binary operation -** Apply basic binary operations(sum, multiply, etc) on values in a single row from two selected fields.
- **Unary operation -** Apply basic unary operations(abs, exp, floor, etc) on values in a single row from a selected field.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
- **Binary operation -** Apply basic binary operations(sum, multiply, etc) on values in a single row from two selected fields.
- **Unary operation -** Apply basic unary operations(abs, exp, floor, etc) on values in a single row from a selected field.
- **Binary operation -** Apply basic binary operations (for example, sum or multiply) on values in a single row from two selected fields.
- **Unary operation -** Apply basic unary operations (for example, abs, exp, or floor) on values in a single row from a selected field.

- **Row index -** Insert a field with the row index.
- **Field name -** Select the names of fields you want to use in the calculation for the new field.
- **Calculation -** If you select **Reduce row** mode, then the **Calculation** field appears. Click in the field to see a list of calculation choices you can use to create the new field. For information about available calculations, refer to [Calculation types][].
- **Operation -** If you select **Binary option** mode, then the **Operation** fields appear. These fields allow you to do basic math operations on values in a single row from two selected fields. You can also use numerical values for binary operations.
- **Operation -** If you select **Binary operation** or **Unary operation** mode, then the **Operation** fields appear. These fields allow you to do basic math operations on values in a single row from selected fields. You can also use numerical values for binary operations.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
- **Operation -** If you select **Binary operation** or **Unary operation** mode, then the **Operation** fields appear. These fields allow you to do basic math operations on values in a single row from selected fields. You can also use numerical values for binary operations.
- **Operation -** If you select **Binary operation** or **Unary operation** mode, then the **Operation** fields appear. These fields allow you to apply basic math operations on values in a single row from selected fields. You can also use numerical values for binary operations.

Copy link
Collaborator

@codeincarnate codeincarnate left a comment

Choose a reason for hiding this comment

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

Looks great! See my comments.

@@ -108,11 +108,12 @@ Use this transformation to add a new field calculated from two other fields. Eac

- **Mode -** Select a mode:
- **Reduce row -** Apply selected calculation on each row of selected fields independently.
- **Binary option -** Apply basic math operation(sum, multiply, etc) on values in a single row from two selected fields.
- **Binary operation -** Apply basic binary operations(sum, multiply, etc) on values in a single row from two selected fields.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is maybe a bit beyond the context of this PR but should it seems like we should include language about what these operators are. I'm assuming plenty of folks will know, but I'm guessing there are those that may be confused. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree! I've added explanations to each of the new functions added. I think binary operations should be fine as they are since they are pretty self explanatory. @imatwawana can I please ask for another review of the docs with these new modifications? Thank you!

packages/grafana-data/src/utils/unaryOperators.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@oscarkilhed oscarkilhed left a comment

Choose a reason for hiding this comment

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

Nice! 🎉

Copy link
Collaborator

@imatwawana imatwawana left a comment

Choose a reason for hiding this comment

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

Small wording tweak but otherwise, looks good!

Comment on lines 113 to 117
- **Absolute value (abs)** - Returns the absolute value of the given expression. It represents it's distance from zero as a positive number.
- **Natural exponential (exp)** - Returns _e_ raised to the power of the given expression.
- **Natural logarithm (ln)** - Returns the natural logarithm of a given expression.
- **Floor (floor)** - Returns the largest integer less than or equal to the given expression.
- **Ceiling (ceil)** - Returns the smallest integer greater than or equal to the given expression.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
- **Absolute value (abs)** - Returns the absolute value of the given expression. It represents it's distance from zero as a positive number.
- **Natural exponential (exp)** - Returns _e_ raised to the power of the given expression.
- **Natural logarithm (ln)** - Returns the natural logarithm of a given expression.
- **Floor (floor)** - Returns the largest integer less than or equal to the given expression.
- **Ceiling (ceil)** - Returns the smallest integer greater than or equal to the given expression.
- **Absolute value (abs)** - Returns the absolute value of a given expression. It represents its distance from zero as a positive number.
- **Natural exponential (exp)** - Returns _e_ raised to the power of a given expression.
- **Natural logarithm (ln)** - Returns the natural logarithm of a given expression.
- **Floor (floor)** - Returns the largest integer less than or equal to a given expression.
- **Ceiling (ceil)** - Returns the smallest integer greater than or equal to a given expression.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Much appreciated! Thank you!

Copy link
Collaborator

@codeincarnate codeincarnate left a comment

Choose a reason for hiding this comment

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

Looks great!

@codeincarnate codeincarnate merged commit 8e57691 into main Oct 6, 2023
20 checks passed
@codeincarnate codeincarnate deleted the mdvictor/unary-operations-transform branch October 6, 2023 14:44
@zerok zerok modified the milestones: 10.2.x, 10.2.0 Oct 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature: Add "absolute" as a Binary transform math operator
5 participants