Skip to content

Commit

Permalink
ArgumentParser check_config method and skip_check parameter renamed t…
Browse files Browse the repository at this point in the history 
…o validate and skip_validation (#639)
  • Loading branch information
@mauvilsa
mauvilsa authored Dec 26, 2024
1 parent ef52e40 commit acf1292
Show file tree
Hide file tree
Showing 7 changed files with 143 additions and 53 deletions.
13 changes: 12 additions & 1 deletion CHANGELOG.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,14 @@ Added

Changed
^^^^^^^

- ``jsonargparse.CLI`` renamed to ``jsonargparse.auto_cli`` to follow `PEP 8
<https://peps.python.org/pep-0008/#function-and-variable-names>`__ functions
naming convention (`#640
<https://github.com/omni-us/jsonargparse/pull/640>`__).
- ``ArgumentParser.check_config`` renamed to ``validate`` and ``skip_check``
parameter of ``ArgumentParser.{dump, save, get_defaults}`` renamed to
``skip_validation`` (`#639
<https://github.com/omni-us/jsonargparse/pull/639>`__).

Fixed
^^^^^
Expand All @@ -39,6 +42,13 @@ Fixed
- Nested dataclass with ``init=False`` not working correctly (`#650
<https://github.com/omni-us/jsonargparse/pull/650>`__).

Deprecated
^^^^^^^^^^
- ``ArgumentParser.check_config`` and ``skip_check`` parameter of
``ArgumentParser.{dump, save, get_defaults}`` are deprecated and will be
removed in v5.0.0, instead use ``validate`` and ``skip_validation`` (`#639
<https://github.com/omni-us/jsonargparse/pull/639>`__).


v4.35.0 (2024-12-16)
--------------------
Expand Down Expand Up @@ -71,6 +81,7 @@ Deprecated
config argument as ``--print_%s`` instead of being always ``--print_config``
(`#630 <https://github.com/omni-us/jsonargparse/pull/630>`__).


v4.34.1 (2024-12-02)
--------------------

Expand Down
10 changes: 5 additions & 5 deletions jsonargparse/_actions.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ def apply_config(parser, cfg, dest, value) -> None:
from ._link_arguments import skip_apply_links

with _ActionSubCommands.not_single_subcommand(), previous_config_context(cfg), skip_apply_links():
kwargs = {"env": False, "defaults": False, "_skip_check": True, "_fail_no_subcommand": False}
kwargs = {"env": False, "defaults": False, "_skip_validation": True, "_fail_no_subcommand": False}
try:
cfg_path: Optional[Path] = Path(value, mode=get_config_read_mode())
except TypeError as ex_path:
Expand Down Expand Up @@ -254,7 +254,7 @@ def __init__(
)

def __call__(self, parser, namespace, value, option_string=None):
kwargs = {"subparser": parser, "key": None, "skip_none": False, "skip_check": False}
kwargs = {"subparser": parser, "key": None, "skip_none": False, "skip_validation": False}
valid_flags = {"": None, "comments": "yaml_comments", "skip_default": "skip_default", "skip_null": "skip_none"}
if value is not None:
flags = value[0].split(",")
Expand Down Expand Up @@ -666,7 +666,7 @@ def __call__(self, parser, namespace, values, option_string=None):
if subcommand in self._name_parser_map:
subparser = self._name_parser_map[subcommand]
subnamespace = namespace.get(subcommand).clone() if subcommand in namespace else None
kwargs = dict(_skip_check=True, **parse_kwargs.get())
kwargs = dict(_skip_validation=True, **parse_kwargs.get())
namespace[subcommand] = subparser.parse_args(arg_strings, namespace=subnamespace, **kwargs)

@staticmethod
Expand Down Expand Up @@ -779,9 +779,9 @@ def handle_subcommands(
key = prefix + subcommand
with parent_parsers_context(key, parser):
if env:
subnamespace = subparser.parse_env(defaults=defaults, _skip_check=True)
subnamespace = subparser.parse_env(defaults=defaults, _skip_validation=True)
elif defaults:
subnamespace = subparser.get_defaults(skip_check=True)
subnamespace = subparser.get_defaults(skip_validation=True)

# Update all subcommand settings
if subnamespace is not None:
Expand Down
2 changes: 1 addition & 1 deletion jsonargparse/_completions.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ def get_files_completer():
def argcomplete_namespace(caller, parser, namespace):
if caller == "argcomplete":
namespace.__class__ = __import__("jsonargparse").Namespace
namespace = parser.merge_config(parser.get_defaults(skip_check=True), namespace).as_flat()
namespace = parser.merge_config(parser.get_defaults(skip_validation=True), namespace).as_flat()
return namespace


Expand Down
81 changes: 46 additions & 35 deletions jsonargparse/_core.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
argcomplete_namespace,
handle_completions,
)
from ._deprecated import ParserDeprecations
from ._deprecated import ParserDeprecations, deprecated_skip_check
from ._formatters import DefaultHelpFormatter, empty_help, get_env_var
from ._jsonnet import ActionJsonnet
from ._jsonschema import ActionJsonSchema
Expand Down Expand Up @@ -315,7 +315,7 @@ def _parse_common(
env: Optional[bool],
defaults: bool,
with_meta: Optional[bool],
skip_check: bool,
skip_validation: bool,
skip_required: bool = False,
skip_subcommands: bool = False,
fail_no_subcommand: bool = True,
Expand All @@ -327,7 +327,7 @@ def _parse_common(
env: Whether to merge with the parsed environment, None to use parser's default.
defaults: Whether to merge with the parser's defaults.
with_meta: Whether to include metadata in config object, None to use parser's default.
skip_check: Whether to skip check if configuration is valid.
skip_validation: Whether to skip validation of configuration.
skip_required: Whether to skip check of required arguments.
skip_subcommands: Whether to skip subcommand processing.
fail_no_subcommand: Whether to fail if no subcommand given.
Expand Down Expand Up @@ -355,8 +355,8 @@ def _parse_common(
except Exception as ex:
self.error(str(ex), ex)

if not skip_check:
self.check_config(cfg, skip_required=skip_required)
if not skip_validation:
self.validate(cfg, skip_required=skip_required)

if not (with_meta or (with_meta is None and self._default_meta)):
cfg = strip_meta(cfg)
Expand All @@ -371,7 +371,7 @@ def _parse_defaults_and_environ(
):
cfg = Namespace()
if defaults:
cfg = self.get_defaults(skip_check=True)
cfg = self.get_defaults(skip_validation=True)

if env or (env is None and self._default_env):
if environ is None:
Expand Down Expand Up @@ -409,7 +409,7 @@ def parse_args( # type: ignore[override]
Raises:
ArgumentError: If the parsing fails error and exit_on_error=True.
"""
skip_check = get_private_kwargs(kwargs, _skip_check=False)
skip_validation = get_private_kwargs(kwargs, _skip_validation=False)
return_parser_if_captured(self)
handle_completions(self)

Expand All @@ -436,7 +436,7 @@ def parse_args( # type: ignore[override]
env=env,
defaults=defaults,
with_meta=with_meta,
skip_check=skip_check,
skip_validation=skip_validation,
)

except (TypeError, KeyError) as ex:
Expand Down Expand Up @@ -468,7 +468,7 @@ def parse_object(
Raises:
ArgumentError: If the parsing fails error and exit_on_error=True.
"""
skip_check, skip_required = get_private_kwargs(kwargs, _skip_check=False, _skip_required=False)
skip_validation, skip_required = get_private_kwargs(kwargs, _skip_validation=False, _skip_required=False)

try:
cfg = self._parse_defaults_and_environ(defaults, env)
Expand All @@ -484,7 +484,7 @@ def parse_object(
env=env,
defaults=defaults,
with_meta=with_meta,
skip_check=skip_check,
skip_validation=skip_validation,
skip_required=skip_required,
)

Expand All @@ -507,7 +507,7 @@ def _load_env_vars(self, env: Union[Dict[str, str], os._Environ], defaults: bool
env_val = env[env_var]
if env_val in action.choices:
cfg[action.dest] = subcommand = self._check_value_key(action, env_val, action.dest, cfg)
pcfg = action._name_parser_map[env_val].parse_env(env=env, defaults=defaults, _skip_check=True)
pcfg = action._name_parser_map[env_val].parse_env(env=env, defaults=defaults, _skip_validation=True)
for k, v in vars(pcfg).items():
cfg[subcommand + "." + k] = v
for action in actions:
Expand Down Expand Up @@ -544,7 +544,7 @@ def parse_env(
Raises:
ArgumentError: If the parsing fails error and exit_on_error=True.
"""
skip_check, skip_subcommands = get_private_kwargs(kwargs, _skip_check=False, _skip_subcommands=False)
skip_validation, skip_subcommands = get_private_kwargs(kwargs, _skip_validation=False, _skip_subcommands=False)

try:
cfg = self._parse_defaults_and_environ(defaults, env=True, environ=env)
Expand All @@ -553,10 +553,10 @@ def parse_env(
"env": True,
"defaults": defaults,
"with_meta": with_meta,
"skip_check": skip_check,
"skip_validation": skip_validation,
"skip_subcommands": skip_subcommands,
}
if skip_check:
if skip_validation:
kwargs["fail_no_subcommand"] = False

parsed_cfg = self._parse_common(cfg=cfg, **kwargs)
Expand Down Expand Up @@ -633,7 +633,9 @@ def parse_string(
Raises:
ArgumentError: If the parsing fails error and exit_on_error=True.
"""
skip_check, fail_no_subcommand = get_private_kwargs(kwargs, _skip_check=False, _fail_no_subcommand=True)
skip_validation, fail_no_subcommand = get_private_kwargs(
kwargs, _skip_validation=False, _fail_no_subcommand=True
)

try:
with parser_context(load_value_mode=self.parser_mode):
Expand All @@ -648,7 +650,7 @@ def parse_string(
env=env,
defaults=defaults,
with_meta=with_meta,
skip_check=skip_check,
skip_validation=skip_validation,
fail_no_subcommand=fail_no_subcommand,
)

Expand Down Expand Up @@ -731,9 +733,10 @@ def dump(
format: str = "parser_mode",
skip_none: bool = True,
skip_default: bool = False,
skip_check: bool = False,
skip_validation: bool = False,
yaml_comments: bool = False,
skip_link_targets: bool = True,
**kwargs,
) -> str:
"""Generates a yaml or json string for the given configuration object.
Expand All @@ -743,7 +746,7 @@ def dump(
:func:`.set_dumper`.
skip_none: Whether to exclude entries whose value is None.
skip_default: Whether to exclude entries whose value is the same as the default.
skip_check: Whether to skip parser checking.
skip_validation: Whether to skip parser checking.
yaml_comments: Whether to add help content as comments. ``yaml_comments=True`` implies ``format='yaml'``.
skip_link_targets: Whether to exclude link targets.
Expand All @@ -753,25 +756,26 @@ def dump(
Raises:
TypeError: If any of the values of cfg is invalid according to the parser.
"""
skip_validation = deprecated_skip_check(ArgumentParser.dump, kwargs, skip_validation)
check_valid_dump_format(format)

cfg = strip_meta(cfg)
if skip_link_targets:
ActionLink.strip_link_target_keys(self, cfg)

with parser_context(load_value_mode=self.parser_mode):
if not skip_check:
self.check_config(cfg)
if not skip_validation:
self.validate(cfg)

dump_kwargs = {"skip_check": skip_check, "skip_none": skip_none}
dump_kwargs = {"skip_validation": skip_validation, "skip_none": skip_none}
self._dump_cleanup_actions(cfg, self._actions, dump_kwargs)

cfg_dict = cfg.as_dict()

if skip_default:
defaults = self.get_defaults(skip_check=True)
defaults = self.get_defaults(skip_validation=True)
ActionLink.strip_link_target_keys(self, defaults)
self._dump_cleanup_actions(defaults, self._actions, {"skip_check": True, "skip_none": skip_none})
self._dump_cleanup_actions(defaults, self._actions, {"skip_validation": True, "skip_none": skip_none})
self._dump_delete_default_entries(cfg_dict, defaults.as_dict())

with parser_context(parent_parser=self):
Expand All @@ -797,7 +801,11 @@ def _dump_cleanup_actions(self, cfg, actions, dump_kwargs, prefix=""):
value = cfg.get(action_dest)
if value is not None:
with parser_context(parent_parser=self):
value = action.serialize(value, dump_kwargs=dump_kwargs)
if dump_kwargs.get("skip_validation"):
with suppress(ValueError):
value = action.serialize(value, dump_kwargs=dump_kwargs)
else:
value = action.serialize(value, dump_kwargs=dump_kwargs)
cfg.update(value, action_dest)

def _dump_delete_default_entries(self, subcfg, subdefaults):
Expand Down Expand Up @@ -827,10 +835,11 @@ def save(
path: Union[str, os.PathLike],
format: str = "parser_mode",
skip_none: bool = True,
skip_check: bool = False,
skip_validation: bool = False,
overwrite: bool = False,
multifile: bool = True,
branch: Optional[str] = None,
**kwargs,
) -> None:
"""Writes to file(s) the yaml or json for the given configuration object.
Expand All @@ -840,20 +849,21 @@ def save(
format: The output format: ``'yaml'``, ``'json'``, ``'json_indented'``, ``'parser_mode'`` or ones added via
:func:`.set_dumper`.
skip_none: Whether to exclude entries whose value is None.
skip_check: Whether to skip parser checking.
skip_validation: Whether to skip parser checking.
overwrite: Whether to overwrite existing files.
multifile: Whether to save multiple config files by using the __path__ metas.
Raises:
TypeError: If any of the values of cfg is invalid according to the parser.
"""
skip_validation = deprecated_skip_check(ArgumentParser.save, kwargs, skip_validation)
check_valid_dump_format(format)

def check_overwrite(path):
if not overwrite and os.path.isfile(path.absolute):
raise ValueError(f"Refusing to overwrite existing file: {path.absolute}")

dump_kwargs = {"format": format, "skip_none": skip_none, "skip_check": skip_check}
dump_kwargs = {"format": format, "skip_none": skip_none, "skip_validation": skip_validation}

if fsspec_support:
try:
Expand All @@ -880,9 +890,9 @@ def check_overwrite(path):
cfg = cfg.clone()
ActionLink.strip_link_target_keys(self, cfg)

if not skip_check:
if not skip_validation:
with parser_context(load_value_mode=self.parser_mode):
self.check_config(strip_meta(cfg), branch=branch)
self.validate(strip_meta(cfg), branch=branch)

def save_paths(cfg):
for key in cfg.get_sorted_keys():
Expand Down Expand Up @@ -912,7 +922,7 @@ def save_paths(cfg):

with change_to_path_dir(path_fc), parser_context(parent_parser=self):
save_paths(cfg)
dump_kwargs["skip_check"] = True
dump_kwargs["skip_validation"] = True
with open(path_fc.absolute, "w") as f:
f.write(self.dump(cfg, **dump_kwargs)) # type: ignore[arg-type]

Expand Down Expand Up @@ -961,15 +971,16 @@ def check_suppressed_default():
check_suppressed_default()
return defaults.get(action.dest)

def get_defaults(self, skip_check: bool = False) -> Namespace:
def get_defaults(self, skip_validation: bool = False, **kwargs) -> Namespace:
"""Returns a namespace with all default values.
Args:
skip_check: Whether to skip check if configuration is valid.
skip_validation: Whether to skip validation of defaults.
Returns:
An object with all default values as attributes.
"""
skip_validation = deprecated_skip_check(ArgumentParser.get_defaults, kwargs, skip_validation)
cfg = Namespace()
for action in filter_default_actions(self._actions):
if (
Expand All @@ -993,7 +1004,7 @@ def get_defaults(self, skip_check: bool = False) -> Namespace:
env=False,
defaults=False,
with_meta=None,
skip_check=skip_check,
skip_validation=skip_validation,
skip_required=True,
)
except (TypeError, KeyError, argparse.ArgumentError) as ex:
Expand Down Expand Up @@ -1029,7 +1040,7 @@ def error(self, message: str, ex: Optional[Exception] = None) -> NoReturn:
sys.stderr.write(f"error: {message}\n")
self.exit(2)

def check_config(
def validate(
self,
cfg: Namespace,
skip_none: bool = True,
Expand Down Expand Up @@ -1375,7 +1386,7 @@ def _check_value_key(self, action: argparse.Action, value: Any, key: str, cfg: O
if leaf_key == action.dest:
return value
subparser = action._name_parser_map[leaf_key] # type: ignore[attr-defined]
subparser.check_config(value)
subparser.validate(value)
elif isinstance(action, _ActionConfigLoad):
if isinstance(value, str):
value = action.check_type(value, self)
Expand Down
Loading

0 comments on commit acf1292

Please sign in to comment.