Skip to content

Add MlClientDocumentationIT tests for classification.#47569

Merged
przemekwitek merged 3 commits intoelastic:masterfrom
przemekwitek:classification_docs
Oct 11, 2019
Merged

Add MlClientDocumentationIT tests for classification.#47569
przemekwitek merged 3 commits intoelastic:masterfrom
przemekwitek:classification_docs

Conversation

@przemekwitek
Copy link
Copy Markdown

@przemekwitek przemekwitek commented Oct 4, 2019

This PR enhances client documentation tests with the new classification analysis type:

  • testPutDataFrameAnalytics
  • testEvaluateDataFrame, testEvaluateDataFrame_Classification, testEvaluateDataFrame_Regression

Additionally, it adds basic java rest high-level docs related to classification.

Relates #46735

@przemekwitek przemekwitek force-pushed the classification_docs branch 5 times, most recently from 65152a7 to c8df7f8 Compare October 7, 2019 10:05
@przemekwitek przemekwitek removed the WIP label Oct 7, 2019
@przemekwitek przemekwitek marked this pull request as ready for review October 7, 2019 10:06
@przemekwitek przemekwitek added :ml Machine learning >docs General docs changes v7.5.0 v8.0.0 labels Oct 7, 2019
@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/ml-core (:ml)

@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-docs (>docs)

Copy link
Copy Markdown
Contributor

@szabosteve szabosteve left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown

@droberts195 droberts195 left a comment

Choose a reason for hiding this comment

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

Looks good. I left a few nits and questions. Maybe nothing needs to be changed if it's obvious which of the duplicate class names to use, not possible to link to bookmarks on external sites, and we don't care about full stops at the end of numbered items. But they are things to at least consider.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This Regression class is not fully qualified. But I don't think the doc examples include the imports. So this doesn't make it clear which package to choose when typing Regression into an IDE and it suggests two possible classes that could be imported.

It might be best to rename one of the classes, or else fully qualify the name here as well as where the other one is used in the docs.

(Same for Classification on line 3341.)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done.
I've fully qualified the names Regression and Classification in this file for now.
LMK if you like the idea of renaming Regression to RegressionEvaluation and Classification to ClassificationEvaluation (or maybe have a different idea for naming). Then I could move on with renaming.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Is it possible to link to the #Precision bookmark on this page?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

You mean, instead of wikipedia link, or in addition?
Such a section does not exist yet on our page. Should I add it?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You can link to a specific bookmark on the Wikipedia page like this:

https://en.wikipedia.org/wiki/Precision_and_recall#Precision

I'm not sure it's possible in Asciidoc though. Maybe the # causes a problem. If not don't worry.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Ah, that's what you meant.

Sure, done.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Is it possible to link to the #Recall bookmark on this page?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

See my questions above.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done.

Copy link
Copy Markdown

@droberts195 droberts195 left a comment

Choose a reason for hiding this comment

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

LGTM

I'm happy to merge if the docs team is happy with the numbered lists.

@przemekwitek
Copy link
Copy Markdown
Author

run elasticsearch-ci/packaging-sample-matrix

@przemekwitek
Copy link
Copy Markdown
Author

run elasticsearch-ci/packaging-sample

@przemekwitek przemekwitek merged commit 9b5770d into elastic:master Oct 11, 2019
@przemekwitek przemekwitek deleted the classification_docs branch October 11, 2019 06:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

>docs General docs changes :ml Machine learning v7.5.0 v8.0.0-alpha1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants