From 79c4dc41e8b71d3ec14c0016682a7cc017dfadb5 Mon Sep 17 00:00:00 2001 From: Mauricio Villegas <5780272+mauvilsa@users.noreply.github.com> Date: Thu, 11 Sep 2025 05:49:14 +0200 Subject: [PATCH] Fix misleading error message when a namespace is used in a list comprehension and diverse refactorings. --- .pre-commit-config.yaml | 2 +- CHANGELOG.rst | 2 ++ CONTRIBUTING.rst | 9 ++++----- jsonargparse/_namespace.py | 8 +++++--- jsonargparse/_parameter_resolvers.py | 16 ++++++++-------- jsonargparse_tests/test_dataclasses.py | 23 +++++++++++++---------- jsonargparse_tests/test_namespace.py | 8 +++++++- jsonargparse_tests/test_pydantic.py | 3 +-- jsonargparse_tests/test_stubs_resolver.py | 1 + jsonargparse_tests/test_typehints.py | 4 ++-- 10 files changed, 44 insertions(+), 32 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 7cd953c0..41e22b96 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -86,7 +86,7 @@ repos: if [ "${BUMPVERSION_NEW_VERSION+x}" = "" ]; then echo "$(tput setaf 6) Skipped, only runs when bumping version $(tput sgr0)"; else - CHANGELOG=$(grep -E "^v.+\..+\..+ \(....-..-..\)" CHANGELOG.rst | head -n 1); + CHANGELOG=$(grep -E "^v.+\..+\..+ \(.*\)" CHANGELOG.rst | head -n 1); EXPECTED="v$BUMPVERSION_NEW_VERSION ($(date -u +%Y-%m-%d))"; if [ "$CHANGELOG" != "$EXPECTED" ] && [ $(echo $BUMPVERSION_NEW_VERSION | grep -cE "[0-9.]+(\.dev|rc)[0-9]+") = 0 ]; then if [ $(grep -c "^v$BUMPVERSION_NEW_VERSION " CHANGELOG.rst) = 1 ]; then diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 9aa6599c..5e59c262 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -27,6 +27,8 @@ Fixed `__). - ``dataclass`` with default failing when ``validate_defaults=True`` (`#771 `__). +- Misleading error message when a namespace is used in a list comprehension + (`#772 `__). v4.41.0 (2025-09-04) diff --git a/CONTRIBUTING.rst b/CONTRIBUTING.rst index 65c6fbd6..0360937c 100644 --- a/CONTRIBUTING.rst +++ b/CONTRIBUTING.rst @@ -154,17 +154,16 @@ example: .. code-block:: - v4.28.0 (2024-03-??) + v4.28.0 (unreleased) -------------------- Added ^^^^^ - -If no such section exists, just add it. New sections should include ``-??`` in -the date to illustrate that the release date is not known yet. Have a look at -previous releases to decide under which subsection the new entry should go. If -you are unsure, ask in the pull request. +If no such section exists, just add it with "(unreleased)" instead of a date. +Have a look at previous releases to decide under which subsection the new entry +should go. If you are unsure, ask in the pull request. Please don't open pull requests with breaking changes unless this has been discussed and agreed upon in an issue. diff --git a/jsonargparse/_namespace.py b/jsonargparse/_namespace.py index 19f0d1a7..f5d14c6f 100644 --- a/jsonargparse/_namespace.py +++ b/jsonargparse/_namespace.py @@ -126,6 +126,8 @@ def _parse_key(self, key: str) -> Tuple[str, Optional["Namespace"], str]: Raises: KeyError: When given invalid key. """ + if not isinstance(key, str): + raise NSKeyError(f"Key must be a string, got: {key!r}.") if " " in key: raise NSKeyError(f'Spaces not allowed in keys: "{key}".') key_split = split_key(key) @@ -292,9 +294,9 @@ def update( self[key] = value else: prefix = key + "." if key else "" - for key, val in value.items(): - if not only_unset or prefix + key not in self: - self[prefix + key] = val + for subkey, subval in value.items(): + if not only_unset or prefix + subkey not in self: + self[prefix + subkey] = subval return self def get(self, key: str, default: Any = None) -> Any: diff --git a/jsonargparse/_parameter_resolvers.py b/jsonargparse/_parameter_resolvers.py index bc607f3f..e440a7a7 100644 --- a/jsonargparse/_parameter_resolvers.py +++ b/jsonargparse/_parameter_resolvers.py @@ -710,19 +710,19 @@ def replace_param_default_subclass_specs(self, params: List[ParamData]) -> None: subclass_types = get_subclass_types(param.annotation, callable_return=True) if not (class_type and subclass_types and is_subclass(class_type, subclass_types)): continue - subclass_spec: dict = {"class_path": get_import_path(class_type), "init_args": {}} + default: dict = {"class_path": get_import_path(class_type), "init_args": {}} for kwarg in node.keywords: if kwarg.arg and ast_is_constant(kwarg.value): - subclass_spec["init_args"][kwarg.arg] = ast_get_constant_value(kwarg.value) + default["init_args"][kwarg.arg] = ast_get_constant_value(kwarg.value) else: - subclass_spec.clear() + default.clear() break - if not subclass_spec or len(node.args) - num_positionals > 0: + if not default or len(node.args) - num_positionals > 0: self.log_debug(f"unsupported class instance default: {ast_str(default_node)}") - elif subclass_spec: - if not subclass_spec["init_args"]: - del subclass_spec["init_args"] - param.default = subclass_spec + elif default: + if not default["init_args"]: + del default["init_args"] + param.default = default def get_call_class_type(self, node) -> Optional[type]: names = ast_get_name_and_attrs(getattr(node, "func", None)) diff --git a/jsonargparse_tests/test_dataclasses.py b/jsonargparse_tests/test_dataclasses.py index 40c46995..6ca4facd 100644 --- a/jsonargparse_tests/test_dataclasses.py +++ b/jsonargparse_tests/test_dataclasses.py @@ -646,7 +646,16 @@ def test_class_path_union_mixture_dataclass_and_class(parser, union_type): class DataClassWithAliasType: p1: IntOrString # type: ignore[valid-type] - def test_bare_alias_type(parser): + if annotated: + + @dataclasses.dataclass + class DataClassWithAnnotatedAliasType: + p1: annotated[IntOrString, 1] # type: ignore[valid-type] + + +@pytest.mark.skipif(not type_alias_type, reason="TypeAliasType is required") +class TestTypeAliasType: + def test_bare_alias_type(self, parser): parser.add_argument("--data", type=IntOrString) help_str = get_parser_help(parser) help_str_lines = [line for line in help_str.split("\n") if "type: IntOrString" in line] @@ -657,7 +666,7 @@ def test_bare_alias_type(parser): cfg = parser.parse_args(["--data=3"]) assert cfg.data == 3 - def test_dataclass_with_alias_type(parser): + def test_dataclass_with_alias_type(self, parser): parser.add_argument("--data", type=DataClassWithAliasType) help_str = get_parser_help(parser) help_str_lines = [line for line in help_str.split("\n") if "type: IntOrString" in line] @@ -669,7 +678,7 @@ def test_dataclass_with_alias_type(parser): assert cfg.data.p1 == 3 @pytest.mark.skipif(not annotated, reason="Annotated is required") - def test_annotated_alias_type(parser): + def test_annotated_alias_type(self, parser): parser.add_argument("--data", type=annotated[IntOrString, 1]) help_str = get_parser_help(parser) help_str_lines = [line for line in help_str.split("\n") if "type: Annotated[IntOrString, 1]" in line] @@ -680,14 +689,8 @@ def test_annotated_alias_type(parser): cfg = parser.parse_args(["--data=3"]) assert cfg.data == 3 - if annotated: - - @dataclasses.dataclass - class DataClassWithAnnotatedAliasType: - p1: annotated[IntOrString, 1] # type: ignore[valid-type] - @pytest.mark.skipif(not annotated, reason="Annotated is required") - def test_dataclass_with_annotated_alias_type(parser): + def test_dataclass_with_annotated_alias_type(self, parser): parser.add_argument("--data", type=DataClassWithAnnotatedAliasType) help_str = get_parser_help(parser) # The printable field datatype is not uniform across versions. diff --git a/jsonargparse_tests/test_namespace.py b/jsonargparse_tests/test_namespace.py index 921e68f9..77552f09 100644 --- a/jsonargparse_tests/test_namespace.py +++ b/jsonargparse_tests/test_namespace.py @@ -6,7 +6,7 @@ import pytest from jsonargparse import Namespace, dict_to_namespace -from jsonargparse._namespace import meta_keys +from jsonargparse._namespace import NSKeyError, meta_keys skip_if_no_setattr_insertion_order = pytest.mark.skipif( platform.python_implementation() != "CPython", @@ -151,6 +151,12 @@ def test_values_generator(): assert values == [1, 2, 3, {"x": 4, "y": 5}] +def test_non_str_keys(): + ns = Namespace(a=Namespace(b=Namespace(c=1))) + with pytest.raises(NSKeyError, match="Key must be a string, got: 0"): + [x for x in ns.a.b] + + def test_namespace_from_dict(): dic = {"a": 1, "b": {"c": 2}} ns = Namespace(dic) diff --git a/jsonargparse_tests/test_pydantic.py b/jsonargparse_tests/test_pydantic.py index e2b32fe0..e5b660f4 100644 --- a/jsonargparse_tests/test_pydantic.py +++ b/jsonargparse_tests/test_pydantic.py @@ -249,9 +249,8 @@ def test_pydantic_types(self, valid_value, invalid_value, cast, type_str, monkey assert cast(cfg.model.param) == valid_value dump = json_or_yaml_load(parser.dump(cfg)) assert dump == {"model": {"param": valid_value}} - with pytest.raises(ArgumentError) as ctx: + with pytest.raises(ArgumentError, match='Parser key "model.param"'): parser.parse_args([f"--model.param={invalid_value}"]) - ctx.match("model.param") @pytest.mark.skipif(not pydantic_supports_field_init, reason="Field.init is required") def test_dataclass_field_init_false(self, parser): diff --git a/jsonargparse_tests/test_stubs_resolver.py b/jsonargparse_tests/test_stubs_resolver.py index 9a6e978d..c17051f8 100644 --- a/jsonargparse_tests/test_stubs_resolver.py +++ b/jsonargparse_tests/test_stubs_resolver.py @@ -291,6 +291,7 @@ def test_get_params_complex_function_requests_get(parser): assert ["url", "params"] == list(parser.get_defaults().keys()) help_str = get_parser_help(parser) assert "default: Unknown" in help_str + assert "--cookies.help CLASS_PATH_OR_NAME" in help_str # stubs only resolver tests diff --git a/jsonargparse_tests/test_typehints.py b/jsonargparse_tests/test_typehints.py index f591c10f..37e97412 100644 --- a/jsonargparse_tests/test_typehints.py +++ b/jsonargparse_tests/test_typehints.py @@ -720,7 +720,7 @@ def test_valid_unpack_typeddict(parser, init_args): if test_config["test"]["init_args"].get("b") is None: # parser.dump does not dump null b test_config["test"]["init_args"].pop("b", None) - assert json.dumps({"testclass": test_config}).replace(" ", "") == parser.dump(cfg, format="json") + assert test_config == json.loads(parser.dump(cfg, format="json"))["testclass"] @pytest.mark.skipif(not Unpack, reason="Unpack introduced in python 3.11 or backported in typing_extensions") @@ -743,7 +743,7 @@ def test_valid_inherited_unpack_typeddict(parser, init_args): if test_config["init_args"].get("b") is None: # parser.dump does not dump null b test_config["init_args"].pop("b", None) - assert json.dumps({"testclass": test_config}).replace(" ", "") == parser.dump(cfg, format="json") + assert test_config == json.loads(parser.dump(cfg, format="json"))["testclass"] @pytest.mark.skipif(not Unpack, reason="Unpack introduced in python 3.11 or backported in typing_extensions")