Skip to content

Commit

Permalink
Fix deprecation warnings not printing the correct file and line of code.
Browse files Browse the repository at this point in the history
  • Loading branch information
mauvilsa committed Nov 22, 2023
1 parent d1a6fcd commit f84ac2e
Show file tree
Hide file tree
Showing 6 changed files with 122 additions and 30 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ Fixed
^^^^^
- Confusing error message when adding signature parameters that conflict with
existing arguments.
- Deprecation warnings not printing the correct file and line of code.


v4.27.0 (2023-11-02)
Expand Down
2 changes: 1 addition & 1 deletion jsonargparse/_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -1467,7 +1467,7 @@ def env_prefix(self, env_prefix: Union[bool, str]):
env_prefix_property_none_message,
)

deprecation_warning(ArgumentParser, env_prefix_property_none_message)
deprecation_warning(ArgumentParser, env_prefix_property_none_message, stacklevel=3)
env_prefix = False
elif env_prefix is True:
env_prefix = os.path.splitext(self.prog)[0]
Expand Down
22 changes: 13 additions & 9 deletions jsonargparse/_deprecated.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ class JsonargparseDeprecationWarning(DeprecationWarning):
pass


def deprecation_warning(component, message):
def deprecation_warning(component, message, stacklevel=1):
env_var = os.environ.get("JSONARGPARSE_DEPRECATION_WARNINGS", "").lower()
show_warnings = env_var != "off"
all_warnings = env_var == "all"
Expand All @@ -49,9 +49,9 @@ def deprecation_warning(component, message):
and to disable the warnings set JSONARGPARSE_DEPRECATION_WARNINGS=off.
""",
JsonargparseDeprecationWarning,
stacklevel=1,
stacklevel=stacklevel + 2,
)
warning(message, JsonargparseDeprecationWarning, stacklevel=3)
warning(message, JsonargparseDeprecationWarning, stacklevel=stacklevel + 2)
shown_deprecation_warnings.add(component)


Expand Down Expand Up @@ -370,7 +370,7 @@ def set_url_support(enabled: bool):


def deprecation_warning_cli_return_parser():
deprecation_warning("CLI.__init__.return_parser", cli_return_parser_message)
deprecation_warning("CLI.__init__.return_parser", cli_return_parser_message, stacklevel=2)


logger_property_none_message = """
Expand All @@ -391,8 +391,8 @@ def deprecation_warning_cli_return_parser():
"""


def path_skip_check_deprecation():
deprecation_warning("Path.__init__", path_skip_check_message)
def path_skip_check_deprecation(stacklevel=2):
deprecation_warning("Path.__init__", path_skip_check_message, stacklevel=stacklevel)


path_immutable_attrs_message = """
Expand Down Expand Up @@ -486,8 +486,8 @@ def usage_and_exit_error_handler(parser: "ArgumentParser", message: str) -> None
"""


def deprecation_warning_error_handler():
deprecation_warning("ArgumentParser.error_handler", error_handler_message)
def deprecation_warning_error_handler(stacklevel):
deprecation_warning("ArgumentParser.error_handler", error_handler_message, stacklevel=stacklevel)


class ParserDeprecations:
Expand All @@ -510,7 +510,11 @@ def error_handler(self) -> Optional[Callable[["ArgumentParser", str], None]]:
@error_handler.setter
def error_handler(self, error_handler):
if error_handler is not False:
deprecation_warning_error_handler()
stacklevel = 2
stack = inspect.stack()[1]
if stack.filename.endswith("jsonargparse/_deprecated.py"):
stacklevel = 5
deprecation_warning_error_handler(stacklevel)
if callable(error_handler) or error_handler in {None, False}:
self._error_handler = error_handler
else:
Expand Down
2 changes: 1 addition & 1 deletion jsonargparse/_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -763,7 +763,7 @@ def logger(self, logger: Union[bool, str, dict, logging.Logger]):
if logger is None:
from ._deprecated import deprecation_warning, logger_property_none_message

deprecation_warning((LoggerProperty.logger, None), logger_property_none_message)
deprecation_warning((LoggerProperty.logger, None), logger_property_none_message, stacklevel=2)
logger = False
if not logger and debug_mode_active():
logger = {"level": "DEBUG"}
Expand Down
2 changes: 1 addition & 1 deletion jsonargparse/typing.py
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ def path_type(mode: str, docstring: Optional[str] = None, **kwargs) -> type:
if skip_check:
from ._deprecated import path_skip_check_deprecation

path_skip_check_deprecation()
path_skip_check_deprecation(stacklevel=4)
name += "_skip_check"
key_name += " skip_check"

Expand Down
123 changes: 105 additions & 18 deletions jsonargparse_tests/test_deprecated.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
from jsonargparse._util import LoggerProperty, argument_error
from jsonargparse_tests.conftest import (
get_parser_help,
is_posix,
skip_if_docstring_parser_unavailable,
skip_if_requests_unavailable,
)
Expand All @@ -55,6 +56,17 @@ def suppress_stderr():
yield


source = pathlib.Path(__file__).read_text().splitlines()


def assert_deprecation_warn(warns, message, code):
assert message in str(warns[-1].message)
if code is None:
return
assert pathlib.Path(warns[-1].filename).name == pathlib.Path(__file__).name
assert code in source[warns[-1].lineno - 1]


def test_deprecation_warning():
with catch_warnings(record=True) as w:
message = "Deprecation warning"
Expand All @@ -78,7 +90,11 @@ def test_ActionEnum():
parser = ArgumentParser(exit_on_error=False)
with catch_warnings(record=True) as w:
action = ActionEnum(enum=MyEnum)
assert "ActionEnum was deprecated" in str(w[-1].message)
assert_deprecation_warn(
w,
message="ActionEnum was deprecated",
code="ActionEnum(enum=MyEnum)",
)
parser.add_argument("--enum", action=action, default=MyEnum.C, help="Description")

for val in ["A", "B", "C"]:
Expand Down Expand Up @@ -107,7 +123,11 @@ def test_ActionOperators():
parser = ArgumentParser(prog="app", exit_on_error=False)
with catch_warnings(record=True) as w:
parser.add_argument("--le0", action=ActionOperators(expr=("<", 0)))
assert "ActionOperators was deprecated" in str(w[-1].message)
assert_deprecation_warn(
w,
message="ActionOperators was deprecated",
code='ActionOperators(expr=("<", 0))',
)
parser.add_argument(
"--gt1.a.le4",
action=ActionOperators(expr=[(">", 1.0), ("<=", 4.0)], join="and", type=float),
Expand Down Expand Up @@ -155,7 +175,11 @@ def test_url_support_true():
assert "fr" == get_config_read_mode()
with catch_warnings(record=True) as w:
set_url_support(True)
assert "set_url_support was deprecated" in str(w[-1].message)
assert_deprecation_warn(
w,
message="set_url_support was deprecated",
code="set_url_support(True)",
)
assert "fur" == get_config_read_mode()
set_url_support(False)
assert "fr" == get_config_read_mode()
Expand All @@ -179,7 +203,11 @@ def test_instantiate_subclasses():
cfg = parser.parse_object({"cal": {"class_path": "calendar.Calendar"}})
with catch_warnings(record=True) as w:
cfg_init = parser.instantiate_subclasses(cfg)
assert "instantiate_subclasses was deprecated" in str(w[-1].message)
assert_deprecation_warn(
w,
message="instantiate_subclasses was deprecated",
code="parser.instantiate_subclasses(cfg)",
)
assert isinstance(cfg_init["cal"], Calendar)


Expand All @@ -190,7 +218,11 @@ def function(a1: float):
def test_single_function_cli():
with catch_warnings(record=True) as w:
parser = CLI(function, return_parser=True, set_defaults={"a1": 3.4})
assert "return_parser parameter was deprecated" in str(w[-1].message)
assert_deprecation_warn(
w,
message="return_parser parameter was deprecated",
code="CLI(function, return_parser=True,",
)
assert isinstance(parser, ArgumentParser)


Expand All @@ -205,26 +237,45 @@ def cmd2(a2: str = "X"):
def test_multiple_functions_cli():
with catch_warnings(record=True) as w:
parser = CLI([cmd1, cmd2], return_parser=True, set_defaults={"cmd2.a2": "Z"})
assert "return_parser parameter was deprecated" in str(w[-1].message)
assert_deprecation_warn(
w,
message="return_parser parameter was deprecated",
code="CLI([cmd1, cmd2], return_parser=True,",
)
assert isinstance(parser, ArgumentParser)


def test_logger_property_none():
with catch_warnings(record=True) as w:
LoggerProperty(logger=None)
assert " Setting the logger property to None was deprecated" in str(w[-1].message)
assert_deprecation_warn(
w,
message=" Setting the logger property to None was deprecated",
code="LoggerProperty(logger=None)",
)


def test_env_prefix_none():
with catch_warnings(record=True) as w:
ArgumentParser(env_prefix=None)
assert "env_prefix" in str(w[-1].message)
assert_deprecation_warn(
w,
message="env_prefix",
code="ArgumentParser(env_prefix=None)",
)


def test_error_handler_parameter():
with catch_warnings(record=True) as w:
parser = ArgumentParser(error_handler=usage_and_exit_error_handler)
assert "error_handler was deprecated in v4.20.0" in str(w[-1].message)
code = "ArgumentParser(error_handler=usage_"
if not is_posix:
code = None # for some reason the stack trace differs in windows
assert_deprecation_warn(
w,
message="error_handler was deprecated in v4.20.0",
code=code,
)
assert parser.error_handler == usage_and_exit_error_handler
with suppress_stderr(), pytest.raises(SystemExit), catch_warnings(record=True):
parser.parse_args(["--invalid"])
Expand All @@ -238,7 +289,11 @@ def custom_error_handler(self, message):
parser = ArgumentParser()
with catch_warnings(record=True) as w:
parser.error_handler = custom_error_handler
assert "error_handler was deprecated in v4.20.0" in str(w[-1].message)
assert_deprecation_warn(
w,
message="error_handler was deprecated in v4.20.0",
code="parser.error_handler = custom_error_handler",
)
assert parser.error_handler == custom_error_handler

out = StringIO()
Expand All @@ -259,7 +314,11 @@ def test_parse_as_dict(tmp_cwd):
f.write("{}")
with catch_warnings(record=True) as w:
parser = ArgumentParser(parse_as_dict=True, default_meta=False)
assert "``parse_as_dict`` parameter was deprecated" in str(w[-1].message)
assert_deprecation_warn(
w,
message="``parse_as_dict`` parameter was deprecated",
code="ArgumentParser(parse_as_dict=True,",
)
assert {} == parser.parse_args([])
assert {} == parser.parse_env([])
assert {} == parser.parse_string("{}")
Expand All @@ -283,7 +342,11 @@ def test_ActionPath(tmp_cwd):
parser.add_argument("--cfg", action=ActionConfigFile)
with catch_warnings(record=True) as w:
parser.add_argument("--file", action=ActionPath(mode="fr"))
assert "ActionPath was deprecated" in str(w[-1].message)
assert_deprecation_warn(
w,
message="ActionPath was deprecated",
code='ActionPath(mode="fr")',
)
parser.add_argument("--dir", action=ActionPath(mode="drw"))
parser.add_argument("--files", nargs="+", action=ActionPath(mode="fr"))

Expand Down Expand Up @@ -327,7 +390,11 @@ def test_ActionPath_skip_check(tmp_cwd):
parser = ArgumentParser(exit_on_error=False)
with catch_warnings(record=True) as w:
parser.add_argument("--file", action=ActionPath(mode="fr", skip_check=True))
assert "skip_check parameter of Path was deprecated" in str(w[-1].message)
assert_deprecation_warn(
w,
message="skip_check parameter of Path was deprecated",
code='ActionPath(mode="fr", skip_check=True)',
)
cfg = parser.parse_args(["--file=not-exist"])
assert isinstance(cfg.file, Path)
assert str(cfg.file) == "not-exist"
Expand Down Expand Up @@ -395,7 +462,11 @@ def test_ActionPathList(tmp_cwd):
parser = ArgumentParser(prog="app", exit_on_error=False)
with catch_warnings(record=True) as w:
parser.add_argument("--list", nargs="+", action=ActionPathList(mode="fr", rel="list"))
assert "ActionPathList was deprecated" in str(w[-1].message)
assert_deprecation_warn(
w,
message="ActionPathList was deprecated",
code="ActionPathList(mode=",
)
parser.add_argument("--list_cwd", action=ActionPathList(mode="fr", rel="cwd"))

cfg = parser.parse_args(["--list", list_file])
Expand Down Expand Up @@ -437,7 +508,11 @@ def test_import_import_docstring_parse():
with catch_warnings(record=True) as w:
from jsonargparse.optionals import import_docstring_parse

assert "Only use the public API" in str(w[-1].message)
assert_deprecation_warn(
w,
message="Only use the public API",
code="from jsonargparse.optionals import import_docstring_parse",
)
assert import_docstring_parse is import_docstring_parser


Expand All @@ -447,7 +522,11 @@ def test_import_from_deprecated():
with catch_warnings(record=True) as w:
func = deprecated.set_url_support

assert "Only use the public API" in str(w[-1].message)
assert_deprecation_warn(
w,
message="Only use the public API",
code="func = deprecated.set_url_support",
)
assert func is set_url_support


Expand All @@ -473,14 +552,22 @@ def test_import_from_module(module, attr):
module = import_module(f"jsonargparse.{module}")
with catch_warnings(record=True) as w:
getattr(module, attr)
assert "Only use the public API" in str(w[-1].message)
assert_deprecation_warn(
w,
message="Only use the public API",
code="getattr(module, attr)",
)


@pytest.mark.skipif(not jsonnet_support, reason="jsonnet package is required")
def test_action_jsonnet_ext_vars(parser):
with catch_warnings(record=True) as w:
parser.add_argument("--ext_vars", action=ActionJsonnetExtVars())
assert "ActionJsonnetExtVars was deprecated" in str(w[-1].message)
assert_deprecation_warn(
w,
message="ActionJsonnetExtVars was deprecated",
code="action=ActionJsonnetExtVars()",
)
parser.add_argument("--jsonnet", action=ActionJsonnet(ext_vars="ext_vars"))

cfg = parser.parse_args(["--ext_vars", '{"param": 123}', "--jsonnet", example_2_jsonnet])
Expand Down

0 comments on commit f84ac2e

Please sign in to comment.