Improve error message when defining custom type for variables#5114
Merged
harishsk merged 23 commits intodotnet:masterfrom May 28, 2020
wangyems:wangye/errormessage
Merged
Improve error message when defining custom type for variables#5114harishsk merged 23 commits intodotnet:masterfrom wangyems:wangye/errormessage
harishsk merged 23 commits intodotnet:masterfrom
wangyems:wangye/errormessage
Conversation
harishsk
reviewed
May 11, 2020
harishsk
reviewed
May 11, 2020
Codecov Report
@@ Coverage Diff @@
## master #5114 +/- ##
=========================================
Coverage ? 75.56%
=========================================
Files ? 995
Lines ? 179296
Branches ? 19295
=========================================
Hits ? 135488
Misses ? 38544
Partials ? 5264
|
Codecov Report
@@ Coverage Diff @@
## master #5114 +/- ##
=========================================
Coverage ? 75.80%
=========================================
Files ? 993
Lines ? 181169
Branches ? 19510
=========================================
Hits ? 137330
Misses ? 38535
Partials ? 5304
|
Contributor
|
/azp run #Resolved |
|
Azure Pipelines successfully started running 2 pipeline(s). #Resolved |
harishsk
reviewed
May 12, 2020
harishsk
reviewed
May 12, 2020
harishsk
reviewed
May 27, 2020
harishsk
reviewed
May 27, 2020
harishsk
reviewed
May 27, 2020
harishsk
reviewed
May 27, 2020
harishsk
reviewed
May 27, 2020
antoniovs1029
suggested changes
May 27, 2020
Contributor
There was a problem hiding this comment.
I've left some comments. Also, as discussed offline, I think it'd be useful to add a test replicating the original scenario of the original issue, catching the exception, and actually checking that the exception is now the one you've introduced. Thanks 😄
antoniovs1029
approved these changes
May 28, 2020
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
resolve #4122
The unhelpful message stuff is a bit different from the above link's description. ML.NET throws unhelpful message not because the customer uses the wrong type(different from the type defined in onnx), but defining the variable using the same type as OnnxSequenceType. However, the correct type should be IEnumerable.
This change adds more information to existing exception message and adds a special error message for errors like this when customer carelessly defines a container variable without using container type