Skip to content

Default "prediction_field_name" to (dependent_variable + "_prediction") #48232

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
Merged
przemekwitek merged 3 commits into from
Oct 21, 2019
Merged

Default "prediction_field_name" to (dependent_variable + "_prediction") #48232

przemekwitek merged 3 commits into from
Oct 21, 2019

Conversation

@przemekwitek
Copy link
Contributor

@przemekwitek przemekwitek commented Oct 18, 2019 •
edited
Loading

In line with how are defaults handled in other places (see e.g: DataFrameAnalyticsDest.resultsField) this PR sets the default value of prediction_field_name upon Classification and Regression objects creation.

This way the value of prediction_field_name is directly available to the API user / UI.

Relates #46735

@przemekwitek przemekwitek force-pushed the default_prediction_field_name branch from ab789ab to 2451153 Compare October 18, 2019 10:35
@elasticmachine
Copy link
Collaborator

elasticmachine commented Oct 18, 2019

Pinging @elastic/ml-core (:ml)

@benwtrent benwtrent self-requested a review October 18, 2019 13:13
benwtrent
benwtrent approved these changes Oct 18, 2019
View reviewed changes
Copy link
Member

@benwtrent benwtrent left a comment

Choose a reason for hiding this comment

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

Do we do any checks to verify that there are no field name conflicts between existing fields and these result fields?

@przemekwitek przemekwitek force-pushed the default_prediction_field_name branch from 58f960d to a294134 Compare October 21, 2019 05:29
@przemekwitek
Copy link
Contributor Author

przemekwitek commented Oct 21, 2019

Do we do any checks to verify that there are no field name conflicts between existing fields and these result fields?

FWIU we do protect against DataFrameAnalyticsDest.resultsField (by default set to "ml") being present in the source index here: ExtractedFieldsDetector.checkResultsFieldIsNotPresent.
Prediction field is stored under this resultsField so should not conflict.

@przemekwitek przemekwitek force-pushed the default_prediction_field_name branch from 06dc614 to 055b09c Compare October 21, 2019 07:11
@przemekwitek
Copy link
Contributor Author

przemekwitek commented Oct 21, 2019

run elasticsearch-ci/packaging-sample-matrix

@przemekwitek
Copy link
Contributor Author

przemekwitek commented Oct 21, 2019

run elasticsearch-ci/default-distro
run elasticsearch-ci/bwc

@przemekwitek przemekwitek force-pushed the default_prediction_field_name branch from 055b09c to 4478212 Compare October 21, 2019 08:06
@przemekwitek
Copy link
Contributor Author

przemekwitek commented Oct 21, 2019

run elasticsearch-ci/packaging-sample-matrix

@przemekwitek przemekwitek force-pushed the default_prediction_field_name branch from 4478212 to cdf0745 Compare October 21, 2019 08:28
@przemekwitek
Copy link
Contributor Author

przemekwitek commented Oct 21, 2019

run elasticsearch-ci/packaging-sample-matrix

@przemekwitek przemekwitek merged commit aaec18c into elastic:master Oct 21, 2019
@przemekwitek przemekwitek deleted the default_prediction_field_name branch October 21, 2019 09:19
@przemekwitek przemekwitek mentioned this pull request Oct 21, 2019
Merged
przemekwitek added a commit to przemekwitek/elasticsearch that referenced this pull request Oct 21, 2019
@przemekwitek
Default "prediction_field_name" to (dependent_variable + "_prediction…
0429c60 
…") (elastic#48232)
@przemekwitek przemekwitek mentioned this pull request Oct 21, 2019
Merged
przemekwitek added a commit to przemekwitek/elasticsearch that referenced this pull request Oct 21, 2019
@przemekwitek
Default "prediction_field_name" to (dependent_variable + "_prediction…
93bf497 
…") (elastic#48232)
przemekwitek added a commit that referenced this pull request Oct 21, 2019
@przemekwitek
[7.x] Default "prediction_field_name" to (dependent_variable + "_pred…
1a42e37 
…iction") (#48232) (#48279)
przemekwitek added a commit that referenced this pull request Oct 21, 2019
@przemekwitek
Default "prediction_field_name" to (dependent_variable + "_prediction…
e303683 
…") (#48232) (#48282)
@jakelandis jakelandis removed the v8.0.0 label Jul 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@benwtrent benwtrent benwtrent approved these changes

Assignees

No one assigned

Labels

:ml Machine learning >non-issue v7.5.0 v7.6.0 v8.0.0-alpha1

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@przemekwitek @elasticmachine @benwtrent @jakelandis