Skip to content

Conversation

@kere-nel
Copy link
Contributor

@kere-nel kere-nel commented Feb 25, 2020

No description provided.

@kere-nel kere-nel requested a review from a team as a code owner February 25, 2020 23:51
@kere-nel kere-nel changed the title Added Onnx ValueToKey support for more int types Added Onnx ValueToKey and KeyToValue support for more int types Feb 26, 2020
// LabelEncoder doesn't support mapping int64 -> int64, so values are cast to strings
var castOutput = ctx.AddIntermediateVariable(TextDataViewType.Instance, "castOutput", true);
castNode = ctx.CreateNode("Cast", srcVariableName, castOutput, ctx.GetNodeName(opType), "");
castNode = ctx.CreateNode("Cast", srcVariableName, castOutput, ctx.GetNodeName("Cast"), "");
Copy link
Contributor

Choose a reason for hiding this comment

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

Are the first five lines of each of these blocks the same? If so, can they be refactored better?
Even better, can you refactor all these into a single function that is a wrapper with a generic type around GetTermsAndIds?

Copy link
Contributor

@harishsk harishsk left a comment

Choose a reason for hiding this comment

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

:shipit:

@kere-nel kere-nel merged commit c3d1592 into dotnet:master Feb 26, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Mar 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants