dotnet / dotnet/machinelearning

The curious case of TrainedWrapperEstimatorBase and friends

Open
#2,841 1 comment 0 reactions 0 assignees View on GitHub
area-Core enhancement needs-further-triage Priority:2
Dominant language
C#
Stars
9.4k
Forks
2k
Avg merge
2d 20h
Merged PRs (30d)
11

Description

So, while I was doing another round of internalization, one thing that I internalized was this. (Basically, something to handle the shimming from the now internal `IDataTransform` interface to the new `ITransformer` interface, during the regrettable situations -- thankfully few -- where such a thing is still necessary. So this:

https://github.com/dotnet/machinelearning/blob/63a7654edaddaf0eea6af98ea762b01594d083ee/src/Microsoft.ML.Data/DataLoadSave/TransformWrapper.cs#L20

and this

https://github.com/dotnet/machinelearning/blob/63a7654edaddaf0eea6af98ea762b01594d083ee/src/Microsoft.ML.Data/DataLoadSave/TransformWrapper.cs#L151

Now, that's all fine, but after doing the necessary work it seemed that I could delete the estimator wrapper entirely, but then I see this very intriguing note.

https://github.com/dotnet/machinelearning/blob/63a7654edaddaf0eea6af98ea762b01594d083ee/src/Microsoft.ML.Data/DataView/RowToRowMapperTransform.cs#L50-L56

I do not understand what is going on here. This is not essential -- everything here is internal -- but it seems at least odd. This seems to indicate that this method and the class I wanted to delete have something to do with each other, but as far as I can tell they have nothing to do with each other whatsoever. But then why the comment?

Anyway, there's clearly something odd going on. This is absolutely not critical, but I wanted to register an issue about the oddness, since the code underlying it passes my understanding.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.