Skip to content

Merge ModuleCatalog into ComponentCatalog. #1022

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

Merged
merged 2 commits into from
Sep 25, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 9 additions & 14 deletions src/Microsoft.ML.Core/CommandLine/CmdParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -290,8 +290,6 @@ public sealed class CmdParser
private const int SpaceBeforeParam = 2;
private readonly ErrorReporter _reporter;
private readonly IHost _host;
// REVIEW: _catalog should be part of environment and can be get through _host.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

REVIEW: _catalog should be part of environment and can be get through _host. [](start = 11, length = 76)

How about that. :)

private Lazy<ModuleCatalog> _catalog;

/// <summary>
/// Parses a command line. This assumes that the exe name has been stripped off.
Expand Down Expand Up @@ -506,15 +504,13 @@ public static string ArgumentsUsage(IHostEnvironment env, Type type, object defa
private CmdParser(IHostEnvironment env)
{
_host = env.Register("CmdParser");
_catalog = new Lazy<ModuleCatalog>(() => ModuleCatalog.CreateInstance(_host));
_reporter = Console.Error.WriteLine;
}

private CmdParser(IHostEnvironment env, ErrorReporter reporter)
{
Contracts.AssertValueOrNull(reporter);
_host = env.Register("CmdParser");
_catalog = new Lazy<ModuleCatalog>(() => ModuleCatalog.CreateInstance(_host));
_reporter = reporter;
}

Expand Down Expand Up @@ -742,11 +738,11 @@ private bool ParseArgumentList(ArgumentInfo info, string[] strs, object destinat

if (arg.IsComponentFactory)
{
ModuleCatalog.ComponentInfo component;
ComponentCatalog.ComponentInfo component;
if (IsCurlyGroup(value) && value.Length == 2)
arg.Field.SetValue(destination, null);
else if (!arg.IsCollection &&
_catalog.Value.TryFindComponentCaseInsensitive(arg.Field.FieldType, value, out component))
_host.ComponentCatalog.TryFindComponentCaseInsensitive(arg.Field.FieldType, value, out component))
{
var activator = Activator.CreateInstance(component.ArgumentType);
if (!IsCurlyGroup(value) && i + 1 < strs.Length && IsCurlyGroup(strs[i + 1]))
Expand Down Expand Up @@ -1060,7 +1056,7 @@ private ArgumentHelpStrings[] GetAllHelpStrings(IHostEnvironment env, ArgumentIn

private ArgumentHelpStrings GetHelpStrings(IHostEnvironment env, Argument arg)
{
return new ArgumentHelpStrings(arg.GetSyntaxHelp(), arg.GetFullHelpText(env, this));
return new ArgumentHelpStrings(arg.GetSyntaxHelp(), arg.GetFullHelpText(env));
}

private bool LexFileArguments(string fileName, out string[] arguments)
Expand Down Expand Up @@ -1994,7 +1990,7 @@ private bool ParseValue(CmdParser owner, string data, out object value)
return false;
}

private void AppendHelpValue(IHostEnvironment env, CmdParser owner, StringBuilder builder, object value)
private void AppendHelpValue(IHostEnvironment env, StringBuilder builder, object value)
{
if (value == null)
builder.Append("{}");
Expand All @@ -2006,14 +2002,14 @@ private void AppendHelpValue(IHostEnvironment env, CmdParser owner, StringBuilde
foreach (object o in (System.Array)value)
{
builder.Append(pre);
AppendHelpValue(env, owner, builder, o);
AppendHelpValue(env, builder, o);
pre = ", ";
}
}
else if (value is IComponentFactory)
{
string name;
var catalog = owner._catalog.Value;
var catalog = env.ComponentCatalog;
var type = value.GetType();
bool success = catalog.TryGetComponentShortName(type, out name);
Contracts.Assert(success);
Expand Down Expand Up @@ -2192,8 +2188,7 @@ private string GetString(IHostEnvironment env, object value, StringBuilder buffe
if (value is IComponentFactory)
{
string name;
// TODO: ModuleCatalog should be on env
var catalog = ModuleCatalog.CreateInstance(env);
var catalog = env.ComponentCatalog;
var type = value.GetType();
bool isModuleComponent = catalog.TryGetComponentShortName(type, out name);
if (isModuleComponent)
Expand All @@ -2214,7 +2209,7 @@ private string GetString(IHostEnvironment env, object value, StringBuilder buffe
return value.ToString();
}

public string GetFullHelpText(IHostEnvironment env, CmdParser owner)
public string GetFullHelpText(IHostEnvironment env)
{
if (IsHidden)
return null;
Expand All @@ -2241,7 +2236,7 @@ public string GetFullHelpText(IHostEnvironment env, CmdParser owner)
if (builder.Length > 0)
builder.Append(" ");
builder.Append("Default value:'");
AppendHelpValue(env, owner, builder, DefaultValue);
AppendHelpValue(env, builder, DefaultValue);
builder.Append('\'');
}
if (Utils.Size(ShortNames) != 0)
Expand Down
Loading