Skip to content

Remove TextLoader.Read(string), promote IDataReader<MSS> extension #1797

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
TomFinley opened this issue Nov 30, 2018 · 0 comments
Closed

Remove TextLoader.Read(string), promote IDataReader<MSS> extension #1797

TomFinley opened this issue Nov 30, 2018 · 0 comments
Labels
API Issues pertaining the friendly API good first issue Good for newcomers up-for-grabs A good issue to fix if you are trying to contribute to the project

Comments

@TomFinley
Copy link
Contributor

TomFinley commented Nov 30, 2018

Pursuant to #1090 and #1041 there was agreement that we should be able to just pass a string into a data reader that reads from streams, rather than have to muck about with the creation of this IMultiStreamSource object.

As it happens, this one request was done in two separate ways:

  1. ML Context to create them all #1252 where this instance method on TextLoader was created:

public IDataView Read(string path) => Read(new MultiFileSource(path));

  1. Extension on IDataReader<IMultiStreamSource>, and DataReader<IMultiStreamSource, TShape> to read from one or several file paths, rather than requiring constructing an IMultiStreamSource #1281 where an extension method on top of IDataReader<IMultiStreamSource> was introduced.

Clearly we don't need both, and the second being more flexible and more broadly applicable was the one that should be checked in, so let us remove the class specific method introduced in #1252. The extension method should however be moved to a namespace where it is more likely to be picked up (probably Microsoft.ML) since right now it's a bit buried and generally people would never see this extension method.

@TomFinley TomFinley added good first issue Good for newcomers API Issues pertaining the friendly API up-for-grabs A good issue to fix if you are trying to contribute to the project labels Nov 30, 2018
PaulTFreedman added a commit to PaulTFreedman/machinelearning that referenced this issue Jan 30, 2019
PaulTFreedman added a commit to PaulTFreedman/machinelearning that referenced this issue Feb 3, 2019
@ghost ghost locked as resolved and limited conversation to collaborators Mar 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
API Issues pertaining the friendly API good first issue Good for newcomers up-for-grabs A good issue to fix if you are trying to contribute to the project
Projects
None yet
Development

No branches or pull requests

1 participant