-
Notifications
You must be signed in to change notification settings - Fork 12.3k
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: De-emphasize non-applicable transformations #76373
Conversation
Oops, forgot to put this in draft 😢. Still needs a little to hammer out. |
neat! 📸 |
Nice! Will review in the morning! |
Added a few things:
Currently although they're marked as disabled they can still be used as we don't wish to make these unusable for cases that we haven't taken into account. |
@ryantxu if you could take another look it would be greatly appreciated. I moved to the applicability levels per your suggestion and I think we can get quite a lot from this! Would be great to know your thoughts in regard to the implementation there. |
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 looking good!
There's a few other transformations that don't really work with all kinds of data. TimeSeriesToTable for instance, doesn't work with multiple fields in the same series. But adding and tuning these rules could be left for follow up PRs i guess.
Two thoughts. The transformations are still selectable. I'm a bit torn on this, I think right now they need to be enabled, because there is no way to insert a transformation between two transformations, a transformation could be applicable before one of the applied transformations, but not after it. But it's also a bit weird having a transformation say that it's disabled but you can still apply it. |
Thanks for the comments Oscar!
|
@mdvictor @oscarkilhed if you could take another look at the presentation it could be appreciated 🙏 |
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.
Looks good! 🎉
What is this feature?
This PR adds support to transformations for an
applicator
function which can be used to determine if the transformation is applicable to the current data.Why do we need this feature?
Many transformations have base conditions needed for them to work correctly. Currently however, we don't do any checks to see if these base conditions are met. This makes for a confusing experience, as it's currently only possible to tell through experimentation or from prior knowledge what given transformations will work on a given data set.
Who is this feature for?
Users of transformations
Which issue(s) does this PR fix?:
Fixes #74199
Special notes for your reviewer:
Please check that: