CustomPipelineColumn#1091
Conversation
|
@TomFinley Is this what you meant by
|
| /// <summary> | ||
| /// For representing a custom <see cref="ColumnType"/>. | ||
| /// </summary> | ||
| /// <typeparam name="T"></typeparam> |
There was a problem hiding this comment.
[](start = 8, length = 32)
remove empty #Resolved
|
Hi @Ivanidzo4ka. Note that you must also add this here, so that an analysis instance is actually created. But this is interesting: your build actually passed. I guess no where do we actually create an analysis instance over types. Let's have a test that will fail, since really we should have caught it in a test. This might be a good starting point... The goal is to have some sort of case where we have an analysis instance that includes this new type. I think this would do it: var reader = TextLoader.CreateReader(env,
ctx => ctx.LoadText(0).LoadAsImage());
var est = reader.MakeNewEstimator().Append(r => r.AsGrayscale().Resize(10, 8).ExtractPixels());
reader.Append(est);That makes the test more verbose in what is from a user's perspective kind of pointless, but it would I think catch this problem. |
TomFinley
left a comment
There was a problem hiding this comment.
Thanks @Ivanidzo4ka ! If we could make the analysis instance creation work with this new type, and also perhaps have a test that would have caught the problem, then I'll be super happy with this. 😄
Fixes #1043