-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Rename CreateTextReader to CreateTextLoader #2125
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,33 +11,33 @@ namespace Microsoft.ML | |
public static class TextLoaderSaverCatalog | ||
{ | ||
/// <summary> | ||
/// Create a text reader <see cref="TextLoader"/>. | ||
/// Create a text loader <see cref="TextLoader"/>. | ||
/// </summary> | ||
/// <param name="catalog">The <see cref="DataOperations"/> catalog.</param> | ||
/// <param name="columns">The columns of the schema.</param> | ||
/// <param name="columns">Array of columns <see cref="TextLoader.Column"/> defining the schema.</param> | ||
/// <param name="hasHeader">Whether the file has a header.</param> | ||
/// <param name="separatorChar">The character used as separator between data points in a row. By default the tab character is used as separator.</param> | ||
/// <param name="dataSample">The optional location of a data sample.</param> | ||
public static TextLoader CreateTextReader(this DataOperations catalog, | ||
/// <param name="dataSample">The optional location of a data sample. The sample can be used to infer column names and number of slots in each column.</param> | ||
public static TextLoader CreateTextLoader(this DataOperations catalog, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
My personal take on this is: TextReader is a much better name than TextLoader. I would rather rename the TextLoader class to TextReader.. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is correct, we are replacing the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah. Good point,. Maybe we should actually yname There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar renames to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My point is to be consistent between method's name and returned type, so either on of the following would be good for me: 👍 public static TextLoader CreateTextLoader() public static TextReader CreateTextReader() There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds good. Can't name it reader, guess we have to reuse the old "loader" thing. |
||
TextLoader.Column[] columns, | ||
bool hasHeader = TextLoader.DefaultArguments.HasHeader, | ||
char separatorChar = TextLoader.DefaultArguments.Separator, | ||
IMultiStreamSource dataSample = null) | ||
=> new TextLoader(CatalogUtils.GetEnvironment(catalog), columns, hasHeader, separatorChar, dataSample); | ||
|
||
/// <summary> | ||
/// Create a text reader <see cref="TextLoader"/>. | ||
/// Create a text loader <see cref="TextLoader"/>. | ||
/// </summary> | ||
/// <param name="catalog">The <see cref="DataOperations"/> catalog.</param> | ||
/// <param name="args">Defines the settings of the load operation.</param> | ||
/// <param name="dataSample">Allows to expose items that can be used for reading.</param> | ||
public static TextLoader CreateTextReader(this DataOperations catalog, | ||
/// <param name="dataSample">The optional location of a data sample. The sample can be used to infer column names and number of slots in each column.</param> | ||
public static TextLoader CreateTextLoader(this DataOperations catalog, | ||
TextLoader.Arguments args, | ||
IMultiStreamSource dataSample = null) | ||
=> new TextLoader(CatalogUtils.GetEnvironment(catalog), args, dataSample); | ||
|
||
/// <summary> | ||
/// Create a text reader <see cref="TextLoader"/> by inferencing the dataset schema from a data model type. | ||
/// Create a text loader <see cref="TextLoader"/> by inferencing the dataset schema from a data model type. | ||
/// </summary> | ||
/// <param name="catalog">The <see cref="DataOperations"/> catalog.</param> | ||
/// <param name="hasHeader">Does the file contains header?</param> | ||
|
@@ -51,7 +51,7 @@ public static TextLoader CreateTextReader(this DataOperations catalog, | |
/// if one of the row contains "5 2:6 4:3" that's mean there are 5 columns all zero | ||
/// except for 3rd and 5th columns which have values 6 and 3</param> | ||
/// <param name="trimWhitespace">Remove trailing whitespace from lines</param> | ||
public static TextLoader CreateTextReader<TInput>(this DataOperations catalog, | ||
public static TextLoader CreateTextLoader<TInput>(this DataOperations catalog, | ||
bool hasHeader = TextLoader.DefaultArguments.HasHeader, | ||
char separatorChar = TextLoader.DefaultArguments.Separator, | ||
bool allowQuotedStrings = TextLoader.DefaultArguments.AllowQuoting, | ||
|
Uh oh!
There was an error while loading. Please reload this page.