Skip to content

Commit ed56c76

Browse files
authored
Revert "Cleaning up --help: Artifact Management Subcommands (#886)"
This reverts commit 8c7165e.
1 parent 8c7165e commit ed56c76

File tree

3 files changed

+22
-63
lines changed

3 files changed

+22
-63
lines changed

cli.py

Lines changed: 9 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -24,35 +24,25 @@
2424
).expanduser()
2525

2626

27-
# Subcommands related to downloading and managing model artifacts
28-
INVENTORY_VERBS = ["download", "list", "remove", "where"]
29-
30-
# List of all supported subcommands in torchchat
31-
KNOWN_VERBS = ["chat", "browser", "generate", "eval", "export"] + INVENTORY_VERBS
27+
KNOWN_VERBS = ["chat", "browser", "download", "generate", "eval", "export", "list", "remove", "where"]
3228

3329
# Handle CLI arguments that are common to a majority of subcommands.
3430
def check_args(args, verb: str) -> None:
3531
# Handle model download. Skip this for download, since it has slightly
3632
# different semantics.
3733
if (
38-
verb not in INVENTORY_VERBS
34+
verb not in ["download", "list", "remove"]
3935
and args.model
4036
and not is_model_downloaded(args.model, args.model_directory)
4137
):
4238
download_and_convert(args.model, args.model_directory, args.hf_token)
4339

4440

45-
def add_arguments_for_verb(parser, verb: str) -> None:
41+
def add_arguments_for_verb(parser, verb: str):
4642
# Model specification. TODO Simplify this.
4743
# A model can be specified using a positional model name or HuggingFace
4844
# path. Alternatively, the model can be specified via --gguf-path or via
4945
# an explicit --checkpoint-dir, --checkpoint-path, or --tokenizer-path.
50-
51-
if verb in INVENTORY_VERBS:
52-
_configure_artifact_inventory_args(parser, verb)
53-
_add_cli_metadata_args(parser)
54-
return
55-
5646
parser.add_argument(
5747
"model",
5848
type=str,
@@ -201,6 +191,12 @@ def add_arguments_for_verb(parser, verb: str) -> None:
201191
choices=allowable_dtype_names(),
202192
help="Override the dtype of the model (default is the checkpoint dtype). Options: bf16, fp16, fp32, fast16, fast",
203193
)
194+
parser.add_argument(
195+
"-v",
196+
"--verbose",
197+
action="store_true",
198+
help="Verbose output",
199+
)
204200
parser.add_argument(
205201
"--quantize",
206202
type=str,
@@ -256,45 +252,6 @@ def add_arguments_for_verb(parser, verb: str) -> None:
256252
default=5000,
257253
help="Port for the web server in browser mode",
258254
)
259-
_add_cli_metadata_args(parser)
260-
261-
262-
# Add CLI Args that are relevant to any subcommand execution
263-
def _add_cli_metadata_args(parser) -> None:
264-
parser.add_argument(
265-
"-v",
266-
"--verbose",
267-
action="store_true",
268-
help="Verbose output",
269-
)
270-
271-
# Configure CLI Args specific to Model Artifact Management
272-
def _configure_artifact_inventory_args(parser, verb: str) -> None:
273-
if verb in ["download", "remove", "where"]:
274-
parser.add_argument(
275-
"model",
276-
type=str,
277-
nargs="?",
278-
default=None,
279-
help="Model name for well-known models",
280-
)
281-
282-
if verb in INVENTORY_VERBS:
283-
parser.add_argument(
284-
"--model-directory",
285-
type=Path,
286-
default=default_model_dir,
287-
help=f"The directory to store downloaded model artifacts. Default: {default_model_dir}",
288-
)
289-
290-
if verb == "download":
291-
parser.add_argument(
292-
"--hf-token",
293-
type=str,
294-
default=None,
295-
help="A HuggingFace API token to use when downloading model artifacts",
296-
)
297-
298255

299256
# Add CLI Args specific to Model Evaluation
300257
def _add_evaluation_args(parser) -> None:

download.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,13 @@ def is_model_downloaded(model: str, models_dir: Path) -> bool:
125125

126126
# Subcommand to list available models.
127127
def list_main(args) -> None:
128+
# TODO It would be nice to have argparse validate this. However, we have
129+
# model as an optional named parameter for all subcommands, so we'd
130+
# probably need to move it to be registered per-command.
131+
if args.model:
132+
print("Usage: torchchat.py list")
133+
return
134+
128135
model_configs = load_model_configs()
129136

130137
# Build the table in-memory so that we can align the text nicely.

torchchat.py

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
from cli import (
1313
add_arguments_for_verb,
1414
KNOWN_VERBS,
15-
INVENTORY_VERBS,
1615
arg_init,
1716
check_args,
1817
)
@@ -50,11 +49,7 @@
5049

5150
# Now parse the arguments
5251
args = parser.parse_args()
53-
54-
# Don't initialize for Inventory management subcommands
55-
# TODO: Remove when arg_init is refactored
56-
if args.command not in INVENTORY_VERBS:
57-
args = arg_init(args)
52+
args = arg_init(args)
5853
logging.basicConfig(
5954
format="%(message)s", level=logging.DEBUG if args.verbose else logging.INFO
6055
)
@@ -75,6 +70,11 @@
7570
from browser.browser import main as browser_main
7671

7772
browser_main(args)
73+
elif args.command == "download":
74+
check_args(args, "download")
75+
from download import download_main
76+
77+
download_main(args)
7878
elif args.command == "generate":
7979
check_args(args, "generate")
8080
from generate import main as generate_main
@@ -89,11 +89,6 @@
8989
from export import main as export_main
9090

9191
export_main(args)
92-
elif args.command == "download":
93-
check_args(args, "download")
94-
from download import download_main
95-
96-
download_main(args)
9792
elif args.command == "list":
9893
check_args(args, "list")
9994
from download import list_main

0 commit comments

Comments
 (0)