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.

Sign up for GitHub

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

Jump to bottom

[ML] Adds the class_assignment_objective to classification #60358

Merged
merged 3 commits into from
Mar 17, 2020
Merged

[ML] Adds the class_assignment_objective to classification #60358

merged 3 commits into from
Mar 17, 2020

Conversation

darnautov
Copy link
Contributor

Summary

Adds a new parameter class_assignment_objective with a default value for classification to detect advanced configuration correctly.
Also fixes the mutation of original job configuration object during the job cloning.

@darnautov darnautov added :ml v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Feature:Data Frame Analytics ML data frame analytics features v7.7.0 labels Mar 17, 2020
@darnautov darnautov requested a review from a team as a code owner March 17, 2020 10:30
@darnautov darnautov self-assigned this Mar 17, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

@darnautov
Merge remote-tracking branch 'upstream/master' into ML-update-dfa-clo…
2648523 
…ning

# Conflicts:
#	x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/hooks/use_create_analytics_form/use_create_analytics_form.ts
walterra
walterra approved these changes Mar 17, 2020
View reviewed changes
Copy link
Contributor

@walterra walterra left a comment

Choose a reason for hiding this comment

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

LGTM, just added a comment about the ternaries in the DeepReadOnly type.

x-pack/plugins/ml/common/types/common.ts
Comment on lines +23 to +29
export type DeepReadonly<T> = T extends Array<infer R>
? ReadonlyArray<DeepReadonly<T>>
: T extends Function
? T
: T extends object
? DeepReadonlyObject<T>
: T;
Copy link
Contributor

Choose a reason for hiding this comment

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

The styleguide speaks out against multiple ternaries here https://github.com/elastic/kibana/blob/master/STYLEGUIDE.md#only-use-ternary-operators-for-small-simple-code

Personally I think it's fine to leave it as is, but maybe you can add a comment that explains the nesting at what each level does?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I'm aware of multiple ternaries in kibana, just used an example from https://www.typescriptlang.org/docs/handbook/advanced-types.html

pheyos
pheyos approved these changes Mar 17, 2020
View reviewed changes
Copy link
Member

@pheyos pheyos left a comment

Choose a reason for hiding this comment

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

Tested and LGTM

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

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

peteharverson
peteharverson approved these changes Mar 17, 2020
View reviewed changes
Copy link
Contributor

@peteharverson peteharverson left a comment

Choose a reason for hiding this comment

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

LGTM

@darnautov darnautov merged commit dd680c7 into elastic:master Mar 17, 2020
@darnautov darnautov deleted the ML-update-dfa-cloning branch March 17, 2020 12:34
@darnautov darnautov mentioned this pull request Mar 17, 2020
Merged
darnautov added a commit to darnautov/kibana that referenced this pull request Mar 17, 2020
@darnautov
[ML] Adds the class_assignment_objective to classification (elastic#6…
6797658 
…0358)

* [ML] add maximize_minimum_recall to classification analysis

* [ML] fix mutation of the original job during the cloning
@pheyos pheyos mentioned this pull request Mar 17, 2020
Closed
darnautov added a commit that referenced this pull request Mar 17, 2020
@darnautov
[ML] Adds the class_assignment_objective to classification (#60358) (#…
c846272 
…60370)

* [ML] add maximize_minimum_recall to classification analysis

* [ML] fix mutation of the original job during the cloning
cnasikas pushed a commit to cnasikas/kibana that referenced this pull request Mar 17, 2020
@darnautov @cnasikas
[ML] Adds the class_assignment_objective to classification (elastic#6…
a70279d 
…0358)

* [ML] add maximize_minimum_recall to classification analysis

* [ML] fix mutation of the original job during the cloning
gmmorris added a commit to gmmorris/kibana that referenced this pull request Mar 17, 2020
@gmmorris
Merge branch 'master' into alerting/tls-warning
08c05b6 
* master:
  [SIEM] Adds 'Closes one signal when more than one opened signals are selected' test again (elastic#60380)
  [SIEM][Detections Engine] - Add rule markdown field to rule create, detail, and edit flows (elastic#60108)
  [Fleet] Add config revision to fleet agents (elastic#60292)
  Allow kbn-config-schema to ignore unknown keys (elastic#59560)
  [ML] Functional tests - disable df analytics clone tests
  skip flaky suite (elastic#58643) (elastic#58991)
  [FTR] Add support for --include and --exclude files via tags (elastic#60123)
  [SIEM] Fix link on overview page (elastic#60348)
  skip flaky test (elastic#60369)
  [Endpoint] Adds take action dropdown and tests to alert details flyout (elastic#59242)
  [Lens] Simplify state management from visualization (elastic#58279)
  Changing default type to start and allowing it to be configured by the event category (elastic#60323)
  [ML] Adds the class_assignment_objective to classification (elastic#60358)
gmmorris added a commit to gmmorris/kibana that referenced this pull request Mar 17, 2020
@gmmorris
Merge branch 'master' into alerting/view-in-app
3c7bea5 
* master: (51 commits)
  do not update cell background if is label cell (elastic#60308)
  FTR configurable test users (elastic#52431)
  [Reporting] Wholesale moves client to newest-platform (elastic#58945)
  [Ingest] Support `show_user` package registry flag (elastic#60338)
  [SIEM] Adds 'Closes one signal when more than one opened signals are selected' test again (elastic#60380)
  [SIEM][Detections Engine] - Add rule markdown field to rule create, detail, and edit flows (elastic#60108)
  [Fleet] Add config revision to fleet agents (elastic#60292)
  Allow kbn-config-schema to ignore unknown keys (elastic#59560)
  [ML] Functional tests - disable df analytics clone tests
  skip flaky suite (elastic#58643) (elastic#58991)
  [FTR] Add support for --include and --exclude files via tags (elastic#60123)
  [SIEM] Fix link on overview page (elastic#60348)
  skip flaky test (elastic#60369)
  [Endpoint] Adds take action dropdown and tests to alert details flyout (elastic#59242)
  [Lens] Simplify state management from visualization (elastic#58279)
  Changing default type to start and allowing it to be configured by the event category (elastic#60323)
  [ML] Adds the class_assignment_objective to classification (elastic#60358)
  [TSVB] fix text color when using custom background color (elastic#60261)
  Fix import to timefilter from in TSVB (elastic#60296)
  [NP] Get rid of usage redirectWhenMissing service (elastic#59777)
  ...
gmmorris added a commit to gmmorris/kibana that referenced this pull request Mar 17, 2020
@gmmorris
Merge branch 'alerting/view-in-app' into alerting/api-constant
1ddeae4 
* alerting/view-in-app: (53 commits)
  fixed typo
  handle optional alerting plugin
  do not update cell background if is label cell (elastic#60308)
  FTR configurable test users (elastic#52431)
  [Reporting] Wholesale moves client to newest-platform (elastic#58945)
  [Ingest] Support `show_user` package registry flag (elastic#60338)
  [SIEM] Adds 'Closes one signal when more than one opened signals are selected' test again (elastic#60380)
  [SIEM][Detections Engine] - Add rule markdown field to rule create, detail, and edit flows (elastic#60108)
  [Fleet] Add config revision to fleet agents (elastic#60292)
  Allow kbn-config-schema to ignore unknown keys (elastic#59560)
  [ML] Functional tests - disable df analytics clone tests
  skip flaky suite (elastic#58643) (elastic#58991)
  [FTR] Add support for --include and --exclude files via tags (elastic#60123)
  [SIEM] Fix link on overview page (elastic#60348)
  skip flaky test (elastic#60369)
  [Endpoint] Adds take action dropdown and tests to alert details flyout (elastic#59242)
  [Lens] Simplify state management from visualization (elastic#58279)
  Changing default type to start and allowing it to be configured by the event category (elastic#60323)
  [ML] Adds the class_assignment_objective to classification (elastic#60358)
  [TSVB] fix text color when using custom background color (elastic#60261)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@walterra walterra walterra approved these changes

@pheyos pheyos pheyos approved these changes

@peteharverson peteharverson peteharverson approved these changes

Assignees

@darnautov darnautov

Labels
Feature:Data Frame Analytics ML data frame analytics features :ml release_note:skip Skip the PR/issue when compiling release notes v7.7.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@darnautov @elasticmachine @kibanamachine @walterra @pheyos @peteharverson