Skip to content

Extend a test for adding string-array output example #1747

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

Merged
merged 4 commits into from
Dec 12, 2018

Conversation

wschin
Copy link
Member

@wschin wschin commented Nov 28, 2018

A partial solution to #1746. At least, one can search the code base to find an example of using strings in input and output.

@wschin wschin added the documentation Related to documentation of ML.NET label Nov 28, 2018
@wschin wschin self-assigned this Nov 28, 2018
var nativeResult = new List<NativeResult>(result.AsEnumerable<NativeResult>(Env, false))[0];

// Check the tokenization of A. Expected result is { "This", "is", "a", "good", "sentence." }.
var tokenizeA = new[] { "This", "is", "a", "good", "sentence." };
Copy link
Contributor

Choose a reason for hiding this comment

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

tokenizeA [](start = 16, length = 9)

it's all cool and nice, but I would rather prefer you to add proper example to https://github.com/dotnet/machinelearning/tree/master/docs/samples/Microsoft.ML.Samples/Dynamic
than modifying this test.

Copy link
Member Author

Choose a reason for hiding this comment

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

We still need a test to test "end-to-end" in terms of C# native data strucutre.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure.
But this test is more about saving/loading in old way, can you move code to Workout test?

And in #1746 you state what you were looking for example, and we usually have examples in Samples project, can you add it to it?


In reply to: 237256819 [](ancestors = 237256819)

Copy link
Member

Choose a reason for hiding this comment

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

+1


In reply to: 237193355 [](ancestors = 237193355)

Copy link
Member Author

Choose a reason for hiding this comment

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

I can but not in this PR. This PR is super low priority.

Copy link
Member

@sfilipi sfilipi left a comment

Choose a reason for hiding this comment

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

:shipit:

Copy link
Contributor

@Ivanidzo4ka Ivanidzo4ka left a comment

Choose a reason for hiding this comment

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

:shipit:

@Ivanidzo4ka
Copy link
Contributor

@wschin Can you either don't close issue #1746 or create separate which would say what we need proper sample (not the test).

@wschin
Copy link
Member Author

wschin commented Dec 12, 2018

@Ivanidzo4ka, sure. Just change this PR's description so that #1746 will not be closed.

@wschin wschin merged commit b49e2b0 into dotnet:master Dec 12, 2018
@wschin wschin deleted the string-array-output-example branch December 12, 2018 16:22
@ghost ghost locked as resolved and limited conversation to collaborators Mar 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation Related to documentation of ML.NET
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants