-
Notifications
You must be signed in to change notification settings - Fork 8.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
[ML] DF Analytics: add results field to wizard and show regression stats #70893
[ML] DF Analytics: add results field to wizard and show regression stats #70893
Conversation
Pinging @elastic/ml-ui (:ml) |
...me_analytics/pages/analytics_exploration/components/regression_exploration/evaluate_stat.tsx
Outdated
Show resolved
Hide resolved
...me_analytics/pages/analytics_exploration/components/regression_exploration/evaluate_stat.tsx
Show resolved
Hide resolved
.../data_frame_analytics/pages/analytics_creation/components/details_step/details_step_form.tsx
Show resolved
Hide resolved
...n/data_frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/reducer.ts
Outdated
Show resolved
Hide resolved
8fa38ad
to
9ae28f1
Compare
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.
Added a couple of comments, but generally LGTM
@@ -414,19 +422,37 @@ export const useRefreshAnalyticsList = ( | |||
|
|||
const DEFAULT_SIG_FIGS = 3; | |||
|
|||
export function getValuesFromResponse(response: RegressionEvaluateResponse) { | |||
let meanSquaredError = response?.regression?.mse?.value; | |||
interface RegressionEvaluteExtractedResponse { |
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.
typo Evalute
}; | ||
|
||
if (response?.regression) { | ||
for (const statType in response.regression) { |
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.
rather than using a for..of, this loop could be written as:
Object.entries(response.regression).forEach(([statType, { value }]) => {
results[statType as keyof RegressionEvaluteExtractedResponse] = value.toPrecision(
DEFAULT_SIG_FIGS
);
});
but i don't know whether it's more readable :)
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.
I prefer the more readable for loop option since in this case there are only 4 properties we're checking for.
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.
Tested latest edits and LGTM (apart from the Evalute
typo!).
9ae28f1
to
8559b4b
Compare
💛 Build succeeded, but was flaky
Test FailuresFirefox UI Functional Tests.test/functional/apps/visualize/_tsvb_chart·ts.visualize app visual builder metric should populate fields for basic functionsStandard Out
Stack Trace
Build metrics
History
To update your PR or re-run it, just comment with: |
…ats (elastic#70893) * add results_field input in form. handle in cloning and editor * update regression evaluate metrics * add additional reg evaluation stats to expanded row and result panel * update jest test * resultsField: check for only spaces string in editor and form * update getValuesFromResult to be less repetitive * update types * fix type name typo
…ats (#70893) (#71161) * add results_field input in form. handle in cloning and editor * update regression evaluate metrics * add additional reg evaluation stats to expanded row and result panel * update jest test * resultsField: check for only spaces string in editor and form * update getValuesFromResult to be less repetitive * update types * fix type name typo
* master: (39 commits) [APM] Add warning to notify user about legacy ML jobs (elastic#71030) updates consumer to siem (elastic#71117) Index pattern creation flow - fix spelling (elastic#71192) [Security Solution][Endpoint] User Manifest Cleanup + Artifact Compression (elastic#70759) [SECURITY] Rearrange rule name's column in Alert Table (elastic#71020) [SECURITY] Alerts back to Detections (elastic#71142) [Security Solution][Exceptions Builder] - Fixes operator selection bug (elastic#71178) [SIEM][Detection Engine] Speeds up value list imports by enabling streaming of files. [APM] Update ML job ID in data telemetry tasks (elastic#71044) [Resolver] Remove `currentPanelView` selector (elastic#71154) add meta.managed to index templates (elastic#71135) Clarify trial subscription levels (elastic#70900) [Security Solution] fix panel links (elastic#71148) skip flaky suite (elastic#69632) skip suite failing ES Promotion (elastic#71018) [ML] DF Analytics: add results field to wizard and show regression stats (elastic#70893) [SIEM] update wordings (elastic#71119) [SECURITY SOLUTION] Rename to hosts and administration (elastic#70913) [ML] Improvements for urlState hook. (elastic#70576) Removing uptime guide (elastic#71124) ...
Summary
Related meta issue: #66661
This PR:
results_field
in job wizardChecklist
Delete any items that are not applicable to this PR.