Skip to content

Commit 353ade5

Browse files
AbishekSfacebook-github-bot
authored andcommitted
Modify runopt alias api to be less verbose
Summary: The changes in previous diffs make the API usages too verbose 1. So in this we revert cfg_key back to a string type and 2. add aliases as list and 3. deprecated_aliases as list. 4. Auto aliasing has been modified as well of type bool, default to be true and only applied to the cfg_key. ``` opts.add( name="rmAttribution", aliases=["tenant"], deprecated_aliases = ["RmAttr"], auto_aliases=True, ) ``` 5. Also modify all the changes made in D85248506 Differential Revision: D85591292
1 parent 5a35922 commit 353ade5

File tree

2 files changed

+52
-104
lines changed

2 files changed

+52
-104
lines changed

torchx/specs/api.py

Lines changed: 33 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -936,11 +936,7 @@ class runopt:
936936
Represents the metadata about the specific run option
937937
"""
938938

939-
class AutoAlias(IntEnum):
940-
snake_case = 0x1
941-
SNAKE_CASE = 0x2
942-
camelCase = 0x4
943-
939+
class AutoAlias:
944940
@staticmethod
945941
def convert_to_camel_case(alias: str) -> str:
946942
words = re.split(r"[_\-\s]+|(?<=[a-z])(?=[A-Z])", alias)
@@ -960,18 +956,12 @@ def convert_to_snake_case(alias: str) -> str:
960956
def convert_to_const_case(alias: str) -> str:
961957
return runopt.AutoAlias.convert_to_snake_case(alias).upper()
962958

963-
class alias(str):
964-
pass
965-
966-
class deprecated(str):
967-
pass
968-
969959
default: CfgVal
970960
opt_type: Type[CfgVal]
971961
is_required: bool
972962
help: str
973-
aliases: set[alias] | None = None
974-
deprecated_aliases: set[deprecated] | None = None
963+
aliases: set[str] | None = None
964+
deprecated_aliases: set[str] | None = None
975965

976966
@property
977967
def is_type_list_of_str(self) -> bool:
@@ -1258,84 +1248,34 @@ def cfg_from_json_repr(self, json_repr: str) -> Dict[str, CfgVal]:
12581248
return cfg
12591249

12601250
def _generate_aliases(
1261-
self, auto_alias: int, aliases: set[str]
1262-
) -> set[runopt.alias]:
1251+
self,
1252+
name: str,
1253+
) -> set[str]:
12631254
generated_aliases = set()
1264-
for alias in aliases:
1265-
if auto_alias & runopt.AutoAlias.camelCase:
1266-
generated_aliases.add(runopt.AutoAlias.convert_to_camel_case(alias))
1267-
if auto_alias & runopt.AutoAlias.snake_case:
1268-
generated_aliases.add(runopt.AutoAlias.convert_to_snake_case(alias))
1269-
if auto_alias & runopt.AutoAlias.SNAKE_CASE:
1270-
generated_aliases.add(runopt.AutoAlias.convert_to_const_case(alias))
1255+
generated_aliases.add(runopt.AutoAlias.convert_to_camel_case(name))
1256+
generated_aliases.add(runopt.AutoAlias.convert_to_snake_case(name))
1257+
generated_aliases.add(runopt.AutoAlias.convert_to_const_case(name))
1258+
generated_aliases.discard(name)
12711259
return generated_aliases
12721260

1273-
def _get_primary_key_and_aliases(
1274-
self,
1275-
cfg_key: list[str | int] | str,
1276-
) -> tuple[str, set[runopt.alias], set[runopt.deprecated]]:
1277-
"""
1278-
Returns the primary key and aliases for the given cfg_key.
1279-
"""
1280-
if isinstance(cfg_key, str):
1281-
return cfg_key, set(), set()
1282-
1283-
if len(cfg_key) == 0:
1284-
raise ValueError("cfg_key must be a non-empty list")
1285-
1286-
if isinstance(cfg_key[0], runopt.alias) or isinstance(
1287-
cfg_key[0], runopt.deprecated
1288-
):
1289-
warnings.warn(
1290-
"The main name of the run option should be the head of the list.",
1291-
UserWarning,
1292-
stacklevel=2,
1293-
)
1294-
primary_key = None
1295-
auto_alias = 0x0
1296-
aliases = set[runopt.alias]()
1297-
deprecated_aliases = set[runopt.deprecated]()
1298-
for name in cfg_key:
1299-
if isinstance(name, runopt.alias):
1300-
aliases.add(name)
1301-
elif isinstance(name, runopt.deprecated):
1302-
deprecated_aliases.add(name)
1303-
elif isinstance(name, int):
1304-
auto_alias = auto_alias | name
1305-
else:
1306-
if primary_key is not None:
1307-
raise ValueError(
1308-
f" Given more than one primary key: {primary_key}, {name}. Please use runopt.alias type for aliases. "
1309-
)
1310-
primary_key = name
1311-
if primary_key is None or primary_key == "":
1312-
raise ValueError(
1313-
"Missing cfg_key. Please provide one other than the aliases."
1314-
)
1315-
if auto_alias != 0x0:
1316-
aliases_to_generate_for = aliases | {primary_key}
1317-
additional_aliases = self._generate_aliases(
1318-
auto_alias, aliases_to_generate_for
1319-
)
1320-
aliases.update(additional_aliases)
1321-
return primary_key, aliases, deprecated_aliases
1322-
13231261
def add(
13241262
self,
1325-
cfg_key: str | list[str | int],
1263+
cfg_key: str,
13261264
type_: Type[CfgVal],
13271265
help: str,
13281266
default: CfgVal = None,
13291267
required: bool = False,
1268+
aliases: Optional[list[str]] = None,
1269+
deprecated_aliases: Optional[list[str]] = None,
1270+
auto_alias: bool = True,
13301271
) -> None:
13311272
"""
13321273
Adds the ``config`` option with the given help string and ``default``
13331274
value (if any). If the ``default`` is not specified then this option
13341275
is a required option.
13351276
"""
1336-
primary_key, aliases, deprecated_aliases = self._get_primary_key_and_aliases(
1337-
cfg_key
1338-
)
1277+
aliases = aliases or []
1278+
deprecated_aliases = deprecated_aliases or []
13391279
if required and default is not None:
13401280
raise ValueError(
13411281
f"Required option: {cfg_key} must not specify default value. Given: {default}"
@@ -1346,12 +1286,24 @@ def add(
13461286
f"Option: {cfg_key}, must be of type: {type_}."
13471287
f" Given: {default} ({type(default).__name__})"
13481288
)
1349-
opt = runopt(default, type_, required, help, aliases, deprecated_aliases)
1350-
for alias in aliases:
1351-
self._alias_to_key[alias] = primary_key
1289+
1290+
deduplicated_aliases = set(aliases)
1291+
if auto_alias:
1292+
deduplicated_aliases.update(self._generate_aliases(cfg_key))
1293+
1294+
opt = runopt(
1295+
default,
1296+
type_,
1297+
required,
1298+
help,
1299+
deduplicated_aliases,
1300+
set(deprecated_aliases),
1301+
)
1302+
for alias in deduplicated_aliases:
1303+
self._alias_to_key[alias] = cfg_key
13521304
for deprecated_alias in deprecated_aliases:
1353-
self._alias_to_key[deprecated_alias] = primary_key
1354-
self._opts[primary_key] = opt
1305+
self._alias_to_key[deprecated_alias] = cfg_key
1306+
self._opts[cfg_key] = opt
13551307

13561308
def update(self, other: "runopts") -> None:
13571309
self._opts.update(other._opts)

torchx/specs/test/api_test.py

Lines changed: 19 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -605,7 +605,8 @@ def test_runopts_add(self) -> None:
605605
def test_runopts_add_with_aliases(self) -> None:
606606
opts = runopts()
607607
opts.add(
608-
["job_priority", runopt.alias("jobPriority")],
608+
"job_priority",
609+
aliases=["jobPriority"],
609610
type_=str,
610611
help="priority for the job",
611612
)
@@ -616,7 +617,8 @@ def test_runopts_add_with_aliases(self) -> None:
616617
def test_runopts_resolve_with_aliases(self) -> None:
617618
opts = runopts()
618619
opts.add(
619-
["job_priority", runopt.alias("jobPriority")],
620+
"job_priority",
621+
aliases=["jobPriority"],
620622
type_=str,
621623
help="priority for the job",
622624
)
@@ -628,12 +630,14 @@ def test_runopts_resolve_with_aliases(self) -> None:
628630
def test_runopts_resolve_with_none_valued_aliases(self) -> None:
629631
opts = runopts()
630632
opts.add(
631-
["job_priority", runopt.alias("jobPriority")],
633+
"job_priority",
634+
aliases=["jobPriority"],
632635
type_=str,
633636
help="priority for the job",
634637
)
635638
opts.add(
636-
["modelTypeName", runopt.alias("model_type_name")],
639+
"modelTypeName",
640+
aliases=["model_type_name"],
637641
type_=Union[str, None],
638642
help="ML Hub Model Type to attribute resource utilization for job",
639643
)
@@ -647,48 +651,40 @@ def test_runopts_resolve_with_none_valued_aliases(self) -> None:
647651

648652
def test_runopts_add_with_deprecated_aliases(self) -> None:
649653
opts = runopts()
650-
with warnings.catch_warnings(record=True) as w:
651-
opts.add(
652-
[runopt.deprecated("jobPriority"), "job_priority"],
653-
type_=str,
654-
help="run as user",
655-
)
656-
self.assertEqual(len(w), 1)
657-
self.assertEqual(w[0].category, UserWarning)
658-
self.assertEqual(
659-
str(w[0].message),
660-
"The main name of the run option should be the head of the list.",
661-
)
654+
opts.add(
655+
"job_priority",
656+
deprecated_aliases=["priority"],
657+
type_=str,
658+
help="run as user",
659+
)
662660

663661
opts.resolve({"job_priority": "high"})
664662
with warnings.catch_warnings(record=True) as w:
665663
warnings.simplefilter("always")
666-
opts.resolve({"jobPriority": "high"})
664+
opts.resolve({"priority": "high"})
667665
self.assertEqual(len(w), 1)
668666
self.assertEqual(w[0].category, UserWarning)
669667
self.assertEqual(
670668
str(w[0].message),
671-
"Run option `jobPriority` is deprecated, use `job_priority` instead",
669+
"Run option `priority` is deprecated, use `job_priority` instead",
672670
)
673671

674672
def test_runopt_auto_aliases(self) -> None:
675673
opts = runopts()
676674
opts.add(
677-
["job_priority", runopt.AutoAlias.camelCase],
675+
"job_priority",
678676
type_=str,
679677
help="run as user",
680678
)
681679
opts.add(
682-
[
683-
"model_type_name",
684-
runopt.AutoAlias.camelCase | runopt.AutoAlias.SNAKE_CASE,
685-
],
680+
"model_type_name",
686681
type_=str,
687682
help="run as user",
688683
)
689684
self.assertEqual(2, len(opts._opts))
690685
self.assertIsNotNone(opts.get("job_priority"))
691686
self.assertIsNotNone(opts.get("jobPriority"))
687+
self.assertIsNotNone(opts.get("JOB_PRIORITY"))
692688
self.assertIsNotNone(opts.get("model_type_name"))
693689
self.assertIsNotNone(opts.get("modelTypeName"))
694690
self.assertIsNotNone(opts.get("MODEL_TYPE_NAME"))

0 commit comments

Comments
 (0)