microsoft / microsoft/winml-cli
Refactor: extract common CLI options into shared decorators
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 40
- Forks
- 11
- Avg merge
- 1d 8h
- Merged PRs (30d)
- 50
Description
Summary
Extract commonly duplicated CLI options (--model, --device, --ep, --task, --output, --verbose) into shared reusable decorators in src/winml/modelkit/utils/cli.py, and adopt them across all commands to eliminate inconsistencies.
Context
Raised in PR #354 review by @xieofxie: "should we add common options for others that are reused across different commands?" — @DingmaomaooBJTU confirmed this should be done.
A shared utility module already exists at src/winml/modelkit/utils/cli.py with four decorators (model_option, ep_option, device_option, verbosity_options), but only analyze.py uses them. The remaining 11 commands each define their own copies of these options with divergent types, defaults, help text, and casing.
Current State
Existing shared decorators in src/winml/modelkit/utils/cli.py
model_option(required=True)—--model/-masclick.Path(exists=True, path_type=Path)ep_option(required=True, optional_message=None)—--epasclick.Choice(ALL_EP_NAMES)device_option(required=True, optional_message=None, default="NPU")—--deviceasclick.Choice(SUPPORTED_DEVICES)verbosity_options(f)— stackable--verbose/-v(count) +--quiet/-q(flag)
Only consumer: analyze.py
All other commands duplicate these options inline.
Key inconsistencies across commands
| Option | Commands using it | Inconsistencies |
|---|---|---|
--model/-m |
analyze, build, compile, eval, export, hub, inspect, perf, quantize (9) | Type varies: click.Path vs str (for HF model IDs); required flag differs |
--device/-d |
analyze, build, compile, config, eval, perf (6) | Default varies: "NPU" (analyze), "npu" (compile), "auto" (config/eval/perf); case sensitivity differs |
--ep |
analyze, build, compile, config, perf (5) | Choice source differs: ALL_EP_NAMES vs VALID_EPS from config module vs manual strings |
--task/-t |
config, eval, export, inspect, perf, quantize (6) | All optional str with auto-detection — duplicated definition |
--output/-o |
analyze, build, compile, config, eval, export, optimize, perf, quantize (9) | Some use --output, others --output-dir; type varies (Path vs str) |
--verbose/-v |
11 of 12 commands | analyze uses stackable count=True; all others use simple is_flag=True |
--precision/-p |
config, perf, quantize (3) | All type=str with similar values; duplicated |
Desired State
- Expand
src/winml/modelkit/utils/cli.pywith additional shared decorators covering all commonly reused options (at minimum:task_option,output_option,precision_option). - Evolve existing decorators to handle the two major model input patterns:
model_path_option— strictclick.Path(exists=True)for commands that require a local.onnxfile (compile, optimize, quantize)model_option— flexiblestrtype for commands that accept HF model IDs or local paths (build, config, eval, export, perf)
- Normalize
--device— unify to a single set of choices (["auto", "cpu", "gpu", "npu"]) withcase_sensitive=Falseand a configurable default (most commands should default to"auto"). - Normalize
--ep— single canonical choice list fromconstants.py; remove manual string lists in individual commands. - Normalize
--verbose— decide whether all commands should use the stackablecountform or the simple flag. If stackable is chosen, adoptverbosity_optionseverywhere. - Adopt shared decorators in all 12 command files — replace inline
@click.option(...)calls with the shared decorator calls. - No behavioral changes — this is a pure refactor; CLI interface and option names must remain backward-compatible.
Acceptance Criteria
- All duplicated options across commands are replaced with shared decorator calls from
cli.py -
--devicechoices and defaults are consistent across all commands -
--epchoice list comes from a single source (constants.py) -
--verbosebehavior is consistent (all stackable or all flag-based) - No CLI-visible breaking changes (option names, short flags, defaults, help text semantics are preserved)
-
uv run ruff check --fixpasses -
uv run pytest tests/passes
Technical Notes
- The
model_optiondecorator currently usesclick.Path(exists=True), which rejects HuggingFace model IDs. Commands likebuild,eval,export, andperfaccept both HF IDs and local paths, so the shared decorator needs atypeparameter or two separate decorators (model_optionfor flexible input,model_path_optionfor strict path validation). compile.pysources its EP choices fromconfig/precision.py:VALID_EPSwhich may differ fromconstants.py:ALL_EP_NAMES. These should be reconciled or the compile command should add its own valid subset via the shared decorator's parameters.- The
verbosity_optionsdecorator adds both--verboseand--quiet, but no command other thananalyzecurrently supports--quiet. Adopting this everywhere would add--quietto all commands — decide if this is desired. - Some commands use
"--model/-m"as a single string (slash syntax), while others use"--model", "-m"as separate args. The shared decorator uses separate args, which is correct Click usage. hub.pyuses-tfor--model-typenot--task, so its short flag conflicts — handle this when adopting sharedtask_option.
Related Files
src/winml/modelkit/utils/cli.py— existing shared decorators (expand here)src/winml/modelkit/utils/constants.py—ALL_EP_NAMES,SUPPORTED_DEVICESsrc/winml/modelkit/commands/analyze.py— only current consumer of shared decorators (reference pattern)src/winml/modelkit/commands/build.py— has its own--ep,--device,--model,--verbosesrc/winml/modelkit/commands/compile.py— has its own--ep,--device,--model,--verbose; sources EP list fromconfig/precision.pysrc/winml/modelkit/commands/config.py— has its own--ep,--device,--model,--task,--verbosesrc/winml/modelkit/commands/eval.py— has its own--device,--model,--task,--output,--verbosesrc/winml/modelkit/commands/export.py— has its own--model,--task,--output,--verbosesrc/winml/modelkit/commands/hub.py— has--model,--output(short flag conflict with--task)src/winml/modelkit/commands/inspect.py— has its own--model,--task,--verbosesrc/winml/modelkit/commands/optimize.py— has its own--model,--output,--verbosesrc/winml/modelkit/commands/perf.py— has its own--ep,--device,--model,--task,--output,--verbose,--precisionsrc/winml/modelkit/commands/quantize.py— has its own--model,--task,--output,--verbose,--precisionsrc/winml/modelkit/commands/sys.py— has its own--verbose
References
- PR review discussion: https://github.com/microsoft/WinML-ModelKit/pull/354
- Click documentation on reusable decorators: https://click.palletsprojects.com/en/stable/options/
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with src/winml/modelkit/utils/cli.py and analyze.py to understand the existing decorator pattern, then compare the option definitions across the command files listed in the issue and the constants in constants.py. Run uv run pytest tests/ and uv run ruff check --fix while checking that all 12 commands retain their option names and compatible behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- cli
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100