Skip to content

Commit 84f36ef

Browse files
AbishekSfacebook-github-bot
authored andcommitted
Add deprecated_aliases to runopt and add warning (#1142)
Summary: Similar to `RunOptAlias` lets introduce and use `RunOptDeprecatedAlias`. This will warn the user with a`UserWarning` when the user uses that specific name and suggests the primary one instead. Differential Revision: D84180061
1 parent d4bcff2 commit 84f36ef

File tree

2 files changed

+51
-6
lines changed

2 files changed

+51
-6
lines changed

torchx/specs/api.py

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
import pathlib
1616
import re
1717
import typing
18+
import warnings
1819
from dataclasses import asdict, dataclass, field
1920
from datetime import datetime
2021
from enum import Enum
@@ -890,6 +891,10 @@ class RunOptAlias(str):
890891
pass
891892

892893

894+
class RunOptDeprecatedAlias(str):
895+
pass
896+
897+
893898
@dataclass
894899
class runopt:
895900
"""
@@ -901,6 +906,7 @@ class runopt:
901906
is_required: bool
902907
help: str
903908
aliases: list[RunOptAlias] | None = None
909+
deprecated_aliases: list[RunOptDeprecatedAlias] | None = None
904910

905911
@property
906912
def is_type_list_of_str(self) -> bool:
@@ -992,7 +998,7 @@ class runopts:
992998

993999
def __init__(self) -> None:
9941000
self._opts: Dict[str, runopt] = {}
995-
self._alias_to_key: Dict[RunOptAlias, str] = {}
1001+
self._alias_to_key: Dict[RunOptAlias | RunOptDeprecatedAlias, str] = {}
9961002

9971003
def __iter__(self) -> Iterator[Tuple[str, runopt]]:
9981004
return self._opts.items().__iter__()
@@ -1043,12 +1049,24 @@ def resolve(self, cfg: Mapping[str, CfgVal]) -> Dict[str, CfgVal]:
10431049
val = resolved_cfg.get(cfg_key)
10441050
resolved_name = None
10451051
aliases = runopt.aliases or []
1052+
deprecated_aliases = runopt.deprecated_aliases or []
10461053
if val is None:
10471054
for alias in aliases:
10481055
val = resolved_cfg.get(alias)
10491056
if val is not None:
10501057
resolved_name = alias
10511058
break
1059+
for alias in deprecated_aliases:
1060+
val = resolved_cfg.get(alias)
1061+
if val is not None:
1062+
resolved_name = alias
1063+
use_instead = self._alias_to_key.get(alias)
1064+
warnings.warn(
1065+
f"Run option: {alias}, is deprecated. Please use {use_instead} instead",
1066+
UserWarning,
1067+
stacklevel=2,
1068+
)
1069+
break
10521070
else:
10531071
resolved_name = cfg_key
10541072
for alias in aliases:
@@ -1174,20 +1192,23 @@ def cfg_from_json_repr(self, json_repr: str) -> Dict[str, CfgVal]:
11741192
def _get_primary_key_and_aliases(
11751193
self,
11761194
cfg_key: list[str] | str,
1177-
) -> tuple[str, list[RunOptAlias]]:
1195+
) -> tuple[str, list[RunOptAlias], list[RunOptDeprecatedAlias]]:
11781196
"""
11791197
Returns the primary key and aliases for the given cfg_key.
11801198
"""
11811199
if isinstance(cfg_key, str):
1182-
return cfg_key, []
1200+
return cfg_key, [], []
11831201

11841202
if len(cfg_key) == 0:
11851203
raise ValueError("cfg_key must be a non-empty list")
11861204
primary_key = None
11871205
aliases = list[RunOptAlias]()
1206+
deprecated_aliases = list[RunOptDeprecatedAlias]()
11881207
for name in cfg_key:
11891208
if isinstance(name, RunOptAlias):
11901209
aliases.append(name)
1210+
elif isinstance(name, RunOptDeprecatedAlias):
1211+
deprecated_aliases.append(name)
11911212
else:
11921213
if primary_key is not None:
11931214
raise ValueError(
@@ -1198,7 +1219,7 @@ def _get_primary_key_and_aliases(
11981219
raise ValueError(
11991220
"Missing cfg_key. Please provide one other than the aliases."
12001221
)
1201-
return primary_key, aliases
1222+
return primary_key, aliases, deprecated_aliases
12021223

12031224
def add(
12041225
self,
@@ -1213,7 +1234,9 @@ def add(
12131234
value (if any). If the ``default`` is not specified then this option
12141235
is a required option.
12151236
"""
1216-
primary_key, aliases = self._get_primary_key_and_aliases(cfg_key)
1237+
primary_key, aliases, deprecated_aliases = self._get_primary_key_and_aliases(
1238+
cfg_key
1239+
)
12171240
if required and default is not None:
12181241
raise ValueError(
12191242
f"Required option: {cfg_key} must not specify default value. Given: {default}"
@@ -1224,9 +1247,11 @@ def add(
12241247
f"Option: {cfg_key}, must be of type: {type_}."
12251248
f" Given: {default} ({type(default).__name__})"
12261249
)
1227-
opt = runopt(default, type_, required, help, aliases)
1250+
opt = runopt(default, type_, required, help, aliases, deprecated_aliases)
12281251
for alias in aliases:
12291252
self._alias_to_key[alias] = primary_key
1253+
for deprecated_alias in deprecated_aliases:
1254+
self._alias_to_key[deprecated_alias] = primary_key
12301255
self._opts[primary_key] = opt
12311256

12321257
def update(self, other: "runopts") -> None:

torchx/specs/test/api_test.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import tempfile
1414
import time
1515
import unittest
16+
import warnings
1617
from dataclasses import asdict
1718
from pathlib import Path
1819
from typing import Dict, List, Mapping, Tuple, Union
@@ -42,6 +43,7 @@
4243
RoleStatus,
4344
runopt,
4445
RunOptAlias,
46+
RunOptDeprecatedAlias,
4547
runopts,
4648
TORCHX_HOME,
4749
Workspace,
@@ -602,6 +604,24 @@ def test_runopts_resolve_with_aliases(self) -> None:
602604
with self.assertRaises(InvalidRunConfigException):
603605
opts.resolve({"job_priority": "high", "jobPriority": "low"})
604606

607+
def test_runopts_add_with_deprecated_aliases(self) -> None:
608+
opts = runopts()
609+
opts.add(
610+
[RunOptDeprecatedAlias("jobPriority"), "job_priority"],
611+
type_=str,
612+
help="run as user",
613+
)
614+
opts.resolve({"job_priority": "high"})
615+
with warnings.catch_warnings(record=True) as w:
616+
warnings.simplefilter("always")
617+
opts.resolve({"jobPriority": "high"})
618+
self.assertEqual(len(w), 1)
619+
self.assertEqual(w[0].category, UserWarning)
620+
self.assertEqual(
621+
str(w[0].message),
622+
"Run option: jobPriority, is deprecated. Please use job_priority instead",
623+
)
624+
605625
def get_runopts(self) -> runopts:
606626
opts = runopts()
607627
opts.add("run_as", type_=str, help="run as user", required=True)

0 commit comments

Comments
 (0)