[Tiny] Use string[] instead of IEnumerable<string> in column names

[wschin]

There are three possible ways to denote input column names, string[], IEnumerable<string>, and params. I personally like string[] because

• IEnumerable<string> is not used elsewhere for the same purpose.
• params makes signature less typed and it forces input column names to be the last argument group.

Fix #2801, Fix #2460.

[TomFinley]

I understand that we wrote our samples this way, but writing it this way means we could not, for instance, take things that are the results of linq queries. It seems like we're just giving up capabilities for no particular benefit.

Why not just standardize elsewhere on IEnumerable?

I think it is important in some cases that we continue to support params when the input column names are reasonably the last item. The syntax just winds up looking cleaner. But those can be overloads I think. #Resolved

[sfilipi]

IEnumerable inputColumnNames, [](start = 10, length = 39)

my vote, and the original issues is to have this be the last parameter of the method, and be a params, to optimize for both cases where the users has one or more columns and wants to provied options.

Having the IEumerable, or with the string[] will require building an Enumerable/Array to hold just one element, which is unnecessary since params exists.

Why would the users have to do either:

var customized_pipeline = ml.Transforms.Text.FeaturizeText(customizedColumnName, options, new List<string> { "SentimentText" }, var customized_pipeline = ml.Transforms.Text.FeaturizeText(customizedColumnName, options, new string[] { "SentimentText" },

when they can just go:

var customized_pipeline = ml.Transforms.Text.FeaturizeText(customizedColumnName, options, "SentimentText"

I do think the signature should be:

public static TextFeaturizingEstimator FeaturizeText(this TransformsCatalog.TextTransforms catalog,
string outputColumnName,
TextFeaturizingEstimator.Options options
params string[] inputColumnNames,
)

and we are doing that everywhere else...

#Resolved

[wschin]

The assumption here is that output column is always single, but why can't it be multiple columns? #Resolved

[sfilipi]

it can be both, hence params is the cleanest

---

In reply to: 261709072 [](ancestors = 261709072)

[codecov]

Codecov Report

Merging #2815 into master will increase coverage by <.01%.
The diff coverage is 18.18%.

@@            Coverage Diff             @@
##           master    #2815      +/-   ##
==========================================
+ Coverage   71.67%   71.68%   +<.01%     
==========================================
  Files         809      808       -1     
  Lines      142416   142392      -24     
  Branches    16120    16112       -8     
==========================================
- Hits       102076   102073       -3     
+ Misses      35910    35886      -24     
- Partials     4430     4433       +3

Flag	Coverage Δ
#Debug	71.68% <18.18%> (ø)	⬆️
#production	67.92% <ø> (+0.01%)	⬆️
#test	85.87% <18.18%> (ø)	⬆️

Impacted Files	Coverage Δ
src/Microsoft.ML.Transforms/Text/TextCatalog.cs	30.43% <ø> (ø)	⬆️
...s/StochasticDualCoordinateAscentClassifierBench.cs	0% <0%> (ø)	⬆️
...ft.ML.Tests/Scenarios/Api/Estimators/Visibility.cs	100% <100%> (ø)	⬆️
src/Microsoft.ML.Maml/MAML.cs	24.75% <0%> (-1.46%)	⬇️
src/Microsoft.ML.TensorFlow/TensorflowTransform.cs	79.76% <0%> (-0.44%)	⬇️
...StandardLearners/Standard/LinearModelParameters.cs	60.63% <0%> (-0.27%)	⬇️
...ML.Transforms/Text/StopWordsRemovingTransformer.cs	85.53% <0%> (-0.17%)	⬇️
...cenariosWithDirectInstantiation/TensorflowTests.cs	91.67% <0%> (-0.09%)	⬇️
...est/Microsoft.ML.StaticPipelineTesting/Training.cs	99.25% <0%> (-0.01%)	⬇️
.../Scenarios/Api/Estimators/IntrospectiveTraining.cs	100% <0%> (ø)	⬆️
... and 14 more

[wschin]

I switch to params now.

---

In reply to: 468742828 [](ancestors = 468742828)

[sfilipi]

[Ivanidzo4ka]

[sharwell]

@wschin This commit appears to have broken the repo build