Skip to content

TextNormalizing export to Onnx #4781

Merged
kere-nel merged 6 commits intodotnet:masterfrom
kere-nel:onnx_text_normalizing
Feb 12, 2020
Merged

TextNormalizing export to Onnx #4781
kere-nel merged 6 commits intodotnet:masterfrom
kere-nel:onnx_text_normalizing

Conversation

@kere-nel
Copy link
Contributor

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

  • can, at the moment, only handle case changing options

@kere-nel kere-nel requested a review from a team as a code owner February 4, 2020 22:17
var isCaseChange = (_parent._caseMode == TextNormalizingEstimator.CaseMode.Lower) ? "LOWER" :
(_parent._caseMode == TextNormalizingEstimator.CaseMode.Upper) ? "UPPER" : "NONE";
node.AddAttribute("case_change_action", isCaseChange);
}
Copy link
Member

Choose a reason for hiding this comment

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

If its 'None' there is no need for the 'StringNormalizer' node at all

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, in the case that we are able to add the other options in the future, it would make sense to keep the option to pass "None". What do you think?

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.

🕐

@kere-nel kere-nel force-pushed the onnx_text_normalizing branch from 9660423 to 2e11361 Compare February 12, 2020 18:43
@kere-nel kere-nel merged commit f5c5876 into dotnet:master Feb 12, 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.

3 participants