[ML]DF Analytics exploration: default filter of results page by `defaultIsTraining` value in url #78303

alvarezmelissa87 · 2020-09-23T15:18:54Z

Summary

Related meta issue: #77182

This PR handles the addition of an defaultIsTraining query param in the url to show a subset of data by default. The relevant analytics exploration pages (classification, regression) will show result table and evaluate panel filtered by the defaultIsTraining value in the url.

Once the default filter has been applied, the defaultIsTraining param is cleared from the URL to prevent confusion.

Example query params: ...ml/data_frame_analytics/exploration?_g=(ml:(analysisType:classification,defaultIsTraining:!t,jobId:flights-class-05))

Checklist

Delete any items that are not applicable to this PR.

Unit or functional tests were updated or added to match the most common scenarios

elasticmachine · 2020-09-23T15:18:57Z

Pinging @elastic/ml-ui (:ml)

peteharverson · 2020-09-23T16:51:33Z

Testing with this bookmarked URL

http://localhost:5601/ulp/app/ml/data_frame_analytics/exploration?_g=(ml:(analysisType:classification,jobId:uci_adult_income_classification,isTraining:false))

The table seems to be filtered correctly, but the label on the Evaluate panel refers to 'entire dataset':

Thanks for taking a look @peteharverson 😄 Updated the default query to filter by training to match what would be produced by the searchBar so that the evaluate panel updates the label correctly. f309d69

darnautov · 2020-09-24T11:03:35Z

x-pack/plugins/ml/common/types/ml_url_generator.ts

@@ -167,6 +167,7 @@ export interface DataFrameAnalyticsExplorationQueryState {
  ml: {
    jobId: JobId;
    analysisType: DataFrameAnalysisConfigType;
+    isTraining?: string;


should it be a boolean instead? 🤔

Yes, good catch. Updated so param is always a boolean. 43a33e3

darnautov · 2020-09-24T11:06:05Z

...ytics/pages/analytics_exploration/components/exploration_query_bar/exploration_query_bar.tsx

@@ -52,7 +52,7 @@ export const ExplorationQueryBar: FC<ExplorationQueryBarProps> = ({
    if (defaultQueryString !== undefined) {
      setSearchInput({ query: defaultQueryString, language: SEARCH_QUERY_LANGUAGE.KUERY });
    }
-  }, []);
+  }, [defaultQueryString !== undefined]);


do you need to perform this check on every update?

This check is required since defaultQueryString will be undefined on first render. I only need it to update once it has a value and then never again.

darnautov · 2020-09-24T11:08:30Z

x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_exploration/page.tsx

+  isTraining?: string | boolean;
+}> = ({ jobId, analysisType, isTraining }) => {
+  if (isTraining !== undefined) {
+    isTraining = isTraining === 'true';
+  }


I think isTraining should always come here as a boolean

Agreed - updated to always be a boolean 43a33e3

darnautov · 2020-09-24T11:35:12Z

TBH I'm not in favor of this implementation using a default query approach. I believe the URL sate should always be a source of truth. At the moment when the user manually changes the query in the search bar, it doesn't persist in the URL state.
Instead, I would update the URL state based on query changes. So the user writes down some query, get a bookmarkable link, and share it with someone. It will also allow filtering on any field, not just isTraining

alvarezmelissa87 · 2020-09-24T14:06:57Z

@darnautov - thanks for taking a look! This is the first step in implementation to unblock the work for the models list. I agree that persisting results view filters in the URL so they can be shared could be very useful but it seems out of scope for this piece of work.

Regarding defaultIsTraining - as it is meant to be a default when you get onto the page - it would make sense to clear it out of the URL. It would also make it more clear that the query is not currently being persisted in the url. So I can go ahead and make that change.

alvarezmelissa87 · 2020-09-24T19:43:25Z

This has been updated with suggested changes and I've made the description clearer. Ready for another look when you get a chance. cc @darnautov, @peteharverson

walterra

LGTM

peteharverson

Tested and LGTM.

The example you give in the PR description needs amending to use defaultIsTraining:!t rather than isTraining:!t I think.

…uce. fixes evaluate panel dataset label

kibanamachine · 2020-09-28T18:39:41Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 907a789

Metrics [docs]

async chunks size

id	value	diff	baseline
ml	8.9MB	+2.6KB	8.9MB

page load bundle size

id	value	diff	baseline
ml	734.8KB	+653.0B	734.1KB

History

💚 Build #77104 succeeded 43a33e3
💚 Build #76980 succeeded 6e19a53
💚 Build #76688 succeeded f309d69
💚 Build #76554 succeeded 158495b

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

…ultIsTraining` value in url (elastic#78303) * df exploration page: handle default isTraining filter in url * default training query updated to match what the searchBar would produce. fixes evaluate panel dataset label * clear defaultIsTraining filter from url once applied

…ultIsTraining` value in url (#78303) (#78670) * df exploration page: handle default isTraining filter in url * default training query updated to match what the searchBar would produce. fixes evaluate panel dataset label * clear defaultIsTraining filter from url once applied

…a into add-anomalies-to-timeline * 'add-anomalies-to-timeline' of github.com:phillipb/kibana: (89 commits) Aligns several module versions across the repository (elastic#78327) Empty prompt and loading spinner for service map (elastic#78382) Change progress bar to spinner (elastic#78460) [QA][Code Coverage] Coverage teams lookup w/o Additional Config (elastic#77111) Slim down core bundle (elastic#75912) [Alerting] retry internal OCC calls within alertsClient (elastic#77838) [kbn/optimizer] only build xpack examples when building xpack plugins (elastic#78656) [Ingest Manager] Ingest setup upgrade (elastic#78081) [Ingest Manager] Surface saved object client 10,000 limitation to bulk actions UI (elastic#78520) fix name without a category or if field end with .text (elastic#78655) [Security Solution] [Detections] Log message enhancements (elastic#78429) [ML]DF Analytics exploration: default filter of results page by `defaultIsTraining` value in url (elastic#78303) [Enterprise Search] Remove all instances of KibanaContext to Kea store (elastic#78513) [ML] DF Analytics creation: ensure job did not fail to start before showing results link (elastic#78200) fix createAppNavigationHandler to use `navigateToUrl` (elastic#78583) Fixing a11y test failure on discover app (elastic#59975) (elastic#77614) [Security Solution] Initiate endpoint package upgrade from security app (elastic#77498) [kbn/es] use a basic build process (elastic#78090) [kbn/optimizer] fix .json extension handling (elastic#78524) Fix APM lodash imports (elastic#78438) ...

* master: (365 commits) making expression debug info serializable (elastic#78727) fix lodahs imports in app-arch code (elastic#78582) Make Field a React.lazy export (elastic#78483) [Security Solution] Improves detections tests (elastic#77295) [TSVB] Different field format on different series is ignored (elastic#78138) RFC: Improve saved object migrations (elastic#66056) [Security Solution] Fixes url timeline flaky test (elastic#78556) adds retryability feature (elastic#78611) Aligns several module versions across the repository (elastic#78327) Empty prompt and loading spinner for service map (elastic#78382) Change progress bar to spinner (elastic#78460) [QA][Code Coverage] Coverage teams lookup w/o Additional Config (elastic#77111) Slim down core bundle (elastic#75912) [Alerting] retry internal OCC calls within alertsClient (elastic#77838) [kbn/optimizer] only build xpack examples when building xpack plugins (elastic#78656) [Ingest Manager] Ingest setup upgrade (elastic#78081) [Ingest Manager] Surface saved object client 10,000 limitation to bulk actions UI (elastic#78520) fix name without a category or if field end with .text (elastic#78655) [Security Solution] [Detections] Log message enhancements (elastic#78429) [ML]DF Analytics exploration: default filter of results page by `defaultIsTraining` value in url (elastic#78303) ...

alvarezmelissa87 added release_note:enhancement :ml Feature:Data Frame Analytics ML data frame analytics features v7.10.0 labels Sep 23, 2020

alvarezmelissa87 requested review from walterra, peteharverson and jgowdyelastic September 23, 2020 15:18

alvarezmelissa87 requested a review from a team as a code owner September 23, 2020 15:18

alvarezmelissa87 self-assigned this Sep 23, 2020

alvarezmelissa87 mentioned this pull request Sep 23, 2020

[ML] DF Analytics Results: ensure is_training filtering works as expected #77182

Closed

5 tasks

alvarezmelissa87 requested review from darnautov and removed request for jgowdyelastic September 23, 2020 16:31

darnautov reviewed Sep 24, 2020

View reviewed changes

alvarezmelissa87 force-pushed the ml-dfanalytics-results-improvements branch from f309d69 to 6e19a53 Compare September 24, 2020 15:19

walterra approved these changes Sep 28, 2020

View reviewed changes

peteharverson approved these changes Sep 28, 2020

View reviewed changes

alvarezmelissa87 added 3 commits September 28, 2020 11:05

df exploration page: handle default isTraining filter in url

d65c1f3

default training query updated to match what the searchBar would prod…

ae49242

…uce. fixes evaluate panel dataset label

clear defaultIsTraining filter from url once applied

907a789

alvarezmelissa87 force-pushed the ml-dfanalytics-results-improvements branch from 43a33e3 to 907a789 Compare September 28, 2020 17:06

alvarezmelissa87 changed the title ~~[ML]DF Analytics exploration: default filter of results page by isTraining value in url~~ [ML]DF Analytics exploration: default filter of results page by defaultIsTraining value in url Sep 28, 2020

alvarezmelissa87 merged commit 25682bf into elastic:master Sep 28, 2020

alvarezmelissa87 mentioned this pull request Sep 28, 2020

[7.x] [ML]DF Analytics exploration: default filter of results page by defaultIsTraining value in url (#78303) #78670

Merged

alvarezmelissa87 deleted the ml-dfanalytics-results-improvements branch September 28, 2020 19:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ML]DF Analytics exploration: default filter of results page by `defaultIsTraining` value in url #78303

[ML]DF Analytics exploration: default filter of results page by `defaultIsTraining` value in url #78303

alvarezmelissa87 commented Sep 23, 2020 •

edited

Loading

elasticmachine commented Sep 23, 2020

peteharverson commented Sep 23, 2020 •

edited by alvarezmelissa87

Loading

darnautov Sep 24, 2020

alvarezmelissa87 Sep 24, 2020

darnautov Sep 24, 2020

alvarezmelissa87 Sep 24, 2020 •

edited

Loading

darnautov Sep 24, 2020

alvarezmelissa87 Sep 24, 2020

darnautov commented Sep 24, 2020

alvarezmelissa87 commented Sep 24, 2020 •

edited

Loading

alvarezmelissa87 commented Sep 24, 2020

walterra left a comment

peteharverson left a comment

kibanamachine commented Sep 28, 2020

[ML]DF Analytics exploration: default filter of results page by defaultIsTraining value in url #78303

[ML]DF Analytics exploration: default filter of results page by defaultIsTraining value in url #78303

Conversation

alvarezmelissa87 commented Sep 23, 2020 • edited Loading

Summary

Checklist

elasticmachine commented Sep 23, 2020

peteharverson commented Sep 23, 2020 • edited by alvarezmelissa87 Loading

darnautov Sep 24, 2020

Choose a reason for hiding this comment

alvarezmelissa87 Sep 24, 2020

Choose a reason for hiding this comment

darnautov Sep 24, 2020

Choose a reason for hiding this comment

alvarezmelissa87 Sep 24, 2020 • edited Loading

Choose a reason for hiding this comment

darnautov Sep 24, 2020

Choose a reason for hiding this comment

alvarezmelissa87 Sep 24, 2020

Choose a reason for hiding this comment

darnautov commented Sep 24, 2020

alvarezmelissa87 commented Sep 24, 2020 • edited Loading

alvarezmelissa87 commented Sep 24, 2020

walterra left a comment

Choose a reason for hiding this comment

peteharverson left a comment

Choose a reason for hiding this comment

kibanamachine commented Sep 28, 2020

💚 Build Succeeded

Metrics [docs]

async chunks size

page load bundle size

History

[ML]DF Analytics exploration: default filter of results page by `defaultIsTraining` value in url #78303

[ML]DF Analytics exploration: default filter of results page by `defaultIsTraining` value in url #78303

alvarezmelissa87 commented Sep 23, 2020 •

edited

Loading

peteharverson commented Sep 23, 2020 •

edited by alvarezmelissa87

Loading

alvarezmelissa87 Sep 24, 2020 •

edited

Loading

alvarezmelissa87 commented Sep 24, 2020 •

edited

Loading