Decide a good name for TextLoader

[najeeb-kazmi]

In #1690 we renamed MLContext.Data.CreateTextReader to MLContext.Data.CreateTextLoader to have the method match the return type TextLoader.

In #581 and subsequent work, we are replacing IDataLoader with IDataReader, and so renaming the TextLoader to TextReader would make sense. However, doing so would lead to disambiguation issues between Microsoft.ML.Data.TextReader and System.IO.TextReader, which we should avoid as per .NET guidelines. So, we must come up with a new name for TextLoader that is descriptive but is different from TextReader.

Some suggestions:

• DelimitedTextReader
• DelimTextReader
• TextDataReader

cc: @glebuk @eerhardt @TomFinley

[jwood803]

Just a random thought, but what about TextFileReader as a suggestion?

[Ivanidzo4ka]

DataViewReader?

[wschin]

DelimitedTextReader sounds good to me.

[artidoro]

Personally, I think that the word 'delimited' in DelimitedTextReader does not provide the most intuitive description of what TextLoader is. Is a single column of text data delimited?

DataViewReader has the problem that there are other readers in ML.NET like the BinaryLoader that are also loading data into a IDataView which could lead to confusion.

I think we need a name that characterizes this loader and that remains simple. That's why I like the idea of keeping the word 'Text' in the name.

I would go for TextDataViewReader as it has the benefits of both suggestions. Another added benefit is that we can make the naming of BinaryLoader consistent with it by renaming it to BinaryDataViewReader, but this is a separate consideration.

What your opinions on this @Ivanidzo4ka, @wschin, @TomFinley, @najeeb-kazmi?

[TomFinley]

I think this was captured somewhere, but I cannot find the issue where we talked about this, so I am going to recapture the gist.

Somewhere else (that I cannot find right now) we determined that IDataReader was a bad name, since TextReader and BinaryReader is already used in .NET, and the potential for confusion was high. We also observed that TextLoader and BinaryLoader had served us (and internal users) well lo these many years.

So, the proposal was: once IDataLoader got internalized (which did happen), we would rename IDataLoader to ILegacyDataLoader (or somesuch), and then we could rename IDataReader and IDataReaderEstimator to IDataLoader and IDataLoaderEstimator.) Since we cannot use reader since .NET BCL already uses that.

[TomFinley]

It may have been something that we discussed in .NET team API review a while ago, where they told us, "don't call it a reader, we don't like that." And we agreed. So maybe like many "obvious" things we had to do we failed to capture it in an issue, since everyone agreed quite readily, to the point where paradoxically it also did not make much of an impression on our minds. 😄

[artidoro]

Makes sense! Thank you for bringing this up here.

I will go ahead and do the change according to the proposal.