Skip to content

Commit

Permalink
Deprecate add_dataclass_arguments and related changes (#634)
Browse files Browse the repository at this point in the history
  • Loading branch information
mauvilsa authored Dec 2, 2024
1 parent 42c1cae commit e9eca4c
Show file tree
Hide file tree
Showing 9 changed files with 138 additions and 128 deletions.
17 changes: 16 additions & 1 deletion CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,24 @@ The semantic versioning only considers the public API as described in
paths are considered internals and can change in minor and patch releases.


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

Changed
^^^^^^^
- Argument groups created from dataclass-like that have zero configurable
arguments no longer adds a config loader (`#634
<https://github.com/omni-us/jsonargparse/pull/634>`__).

Deprecated
^^^^^^^^^^
- ``add_dataclass_arguments`` is deprecated and will be removed in v5.0.0.
Instead use ``add_class_arguments`` (`#634
<https://github.com/omni-us/jsonargparse/pull/634>`__).


v4.34.1 (2024-12-02)

Fixed
^^^^^
- List of dataclass with nested dataclass attribute fails to parse (`#625
Expand Down
6 changes: 1 addition & 5 deletions jsonargparse/_actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from contextvars import ContextVar
from typing import Any, Dict, List, Optional, Tuple, Type, Union

from ._common import Action, get_class_instantiator, is_subclass, parser_context
from ._common import Action, is_subclass, parser_context
from ._loaders_dumpers import get_loader_exceptions, load_value
from ._namespace import Namespace, NSKeyError, split_key, split_key_root
from ._optionals import get_config_read_mode
Expand Down Expand Up @@ -333,10 +333,6 @@ def _load_config(self, value, parser):
def check_type(self, value, parser):
return self._load_config(value, parser)

def instantiate_classes(self, value):
instantiator_fn = get_class_instantiator()
return instantiator_fn(self.basetype, **value)


class _ActionHelpClassPath(Action):
sub_add_kwargs: Dict[str, Any] = {}
Expand Down
54 changes: 29 additions & 25 deletions jsonargparse/_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ def add_argument(self, *args, enable_path: bool = False, **kwargs):
if "type" in kwargs:
if is_dataclass_like(kwargs["type"]):
nested_key = args[0].lstrip("-")
self.add_dataclass_arguments(kwargs.pop("type"), nested_key, **kwargs)
self.add_class_arguments(kwargs.pop("type"), nested_key, **kwargs)
return _find_action(parser, nested_key)
if ActionTypeHint.is_supported_typehint(kwargs["type"]):
args = ActionTypeHint.prepare_add_argument(
Expand Down Expand Up @@ -176,6 +176,33 @@ def add_argument_group(self, *args, name: Optional[str] = None, **kwargs) -> "_A
parser.groups[name] = group # type: ignore[union-attr]
return group

def set_defaults(self, *args: Dict[str, Any], **kwargs: Any) -> None:
"""Sets default values from dictionary or keyword arguments.
Args:
*args: Dictionary defining the default values to set.
**kwargs: Sets default values based on keyword arguments.
Raises:
KeyError: If key not defined in the parser.
"""
for arg in args:
for dest, default in arg.items():
action = _find_action(self, dest)
if action is None:
raise NSKeyError(f'No action for key "{dest}" to set its default.')
elif isinstance(action, ActionConfigFile):
ActionConfigFile.set_default_error()
elif isinstance(action, _ActionConfigLoad):
default = {f"{dest}.{k}": v for k, v in default.items()}
self.set_defaults(default)
continue
if isinstance(action, ActionTypeHint):
default = action.normalize_default(default)
self._defaults[dest] = action.default = default
if kwargs:
self.set_defaults(kwargs)


class _ArgumentGroup(ActionsContainer, argparse._ArgumentGroup):
"""Extension of argparse._ArgumentGroup to support additional functionalities."""
Expand Down Expand Up @@ -884,29 +911,6 @@ def save_paths(cfg):

## Methods related to defaults ##

def set_defaults(self, *args: Dict[str, Any], **kwargs: Any) -> None:
"""Sets default values from dictionary or keyword arguments.
Args:
*args: Dictionary defining the default values to set.
**kwargs: Sets default values based on keyword arguments.
Raises:
KeyError: If key not defined in the parser.
"""
for arg in args:
for dest, default in arg.items():
action = _find_action(self, dest)
if action is None:
raise NSKeyError(f'No action for key "{dest}" to set its default.')
elif isinstance(action, ActionConfigFile):
ActionConfigFile.set_default_error()
if isinstance(action, ActionTypeHint):
default = action.normalize_default(default)
self._defaults[dest] = action.default = default
if kwargs:
self.set_defaults(kwargs)

def _get_default_config_files(self) -> List[Tuple[Optional[str], Path]]:
default_config_files = []

Expand Down Expand Up @@ -1179,7 +1183,7 @@ def instantiate_classes(
cfg = strip_meta(cfg)
for component in components:
ActionLink.apply_instantiation_links(self, cfg, target=component.dest)
if isinstance(component, (ActionTypeHint, _ActionConfigLoad)):
if isinstance(component, ActionTypeHint):
try:
value, parent, key = cfg.get_value_and_parent(component.dest)
except (KeyError, AttributeError):
Expand Down
11 changes: 11 additions & 0 deletions jsonargparse/_deprecated.py
Original file line number Diff line number Diff line change
Expand Up @@ -520,6 +520,17 @@ def error_handler(self, error_handler):
else:
raise ValueError("error_handler can be either a Callable or None.")

@deprecated(
"""
add_dataclass_arguments was deprecated in v4.35.0 and will be removed in
v5.0.0. Instead use add_class_arguments.
"""
)
def add_dataclass_arguments(self, *args, **kwargs):
if "title" in kwargs:
kwargs["help"] = kwargs.pop("title")
return self.add_class_arguments(*args, **kwargs)


ParserError = ArgumentError

Expand Down
95 changes: 25 additions & 70 deletions jsonargparse/_signatures.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import inspect
import re
from argparse import SUPPRESS, ArgumentParser
from contextlib import suppress
from typing import Any, Callable, List, Optional, Set, Tuple, Type, Union

from ._actions import _ActionConfigLoad
Expand All @@ -14,6 +13,7 @@
get_generic_origin,
get_unaliased_type,
is_dataclass_like,
is_final_class,
is_subclass,
)
from ._namespace import Namespace
Expand Down Expand Up @@ -53,7 +53,7 @@ def add_class_arguments(
nested_key: Optional[str] = None,
as_group: bool = True,
as_positional: bool = False,
default: Optional[Union[dict, Namespace, LazyInitBaseClass]] = None,
default: Optional[Union[dict, Namespace, LazyInitBaseClass, Type]] = None,
skip: Optional[Set[Union[str, int]]] = None,
instantiate: bool = True,
fail_untyped: bool = True,
Expand Down Expand Up @@ -82,16 +82,27 @@ def add_class_arguments(
ValueError: When not given a class.
ValueError: When there are required parameters without at least one valid type.
"""
if not inspect.isclass(get_generic_origin(get_unaliased_type(theclass))):
unaliased_class_type = get_unaliased_type(theclass)
if not inspect.isclass(get_generic_origin(unaliased_class_type)):
raise ValueError(f"Expected 'theclass' parameter to be a class type, got: {theclass}")
if not (
isinstance(default, (NoneType, dict, Namespace))
or (isinstance(default, LazyInitBaseClass) and isinstance(default, theclass))
or (isinstance(default, LazyInitBaseClass) and isinstance(default, unaliased_class_type))
or (
not is_final_class(default.__class__)
and is_dataclass_like(default.__class__)
and isinstance(default, unaliased_class_type)
)
):
raise ValueError(
f"Expected 'default' parameter to be a dict, Namespace or lazy instance of the class, got: {default}"
f"Expected 'default' to be dict, Namespace, lazy instance or dataclass-like, got: {default}"
)
linked_targets = get_private_kwargs(kwargs, linked_targets=None)
linked_targets, help, _ = get_private_kwargs(
kwargs,
linked_targets=None,
help=None,
required=None, # Ignored because provided when adding signatures, remove with dataclass inheritance support
)

added_args = self._add_signature_arguments(
theclass,
Expand All @@ -104,6 +115,7 @@ def add_class_arguments(
sub_configs=sub_configs,
instantiate=instantiate,
linked_targets=linked_targets,
help=help,
)

if default:
Expand All @@ -112,6 +124,8 @@ def add_class_arguments(
defaults = default
if isinstance(default, LazyInitBaseClass):
defaults = default.lazy_get_init_args().as_dict()
elif is_dataclass_like(default.__class__):
defaults = dataclass_to_dict(default)
elif isinstance(default, Namespace):
defaults = default.as_dict()
if defaults:
Expand Down Expand Up @@ -230,6 +244,7 @@ def _add_signature_arguments(
sub_configs: bool = False,
instantiate: bool = True,
linked_targets: Optional[Set[str]] = None,
help: Optional[str] = None,
) -> List[str]:
"""Adds arguments from parameters of objects based on signatures and docstrings.
Expand Down Expand Up @@ -273,7 +288,10 @@ def _add_signature_arguments(
)

## Create group if requested ##
doc_group = get_doc_short_description(function_or_class, method_name, logger=self.logger)
if help is not None:
doc_group = help
else:
doc_group = get_doc_short_description(function_or_class, method_name, logger=self.logger)
component = getattr(function_or_class, method_name) if method_name else function_or_class
container = self._create_group_if_requested(
component,
Expand Down Expand Up @@ -424,67 +442,6 @@ def _add_signature_parameter(
f" type. Parameter '{name}' from '{src}' does not specify a type."
)

def add_dataclass_arguments(
self,
theclass: Type,
nested_key: str,
default: Optional[Union[Type, dict]] = None,
as_group: bool = True,
fail_untyped: bool = True,
**kwargs,
) -> List[str]:
"""Adds arguments from a dataclass based on its field types and docstrings.
Args:
theclass: Class from which to add arguments.
nested_key: Key for nested namespace.
default: Value for defaults. Must be instance of or kwargs for theclass.
as_group: Whether arguments should be added to a new argument group.
fail_untyped: Whether to raise exception if a required parameter does not have a type.
Returns:
The list of arguments added.
Raises:
ValueError: When not given a dataclass.
ValueError: When default is not instance of or kwargs for theclass.
"""
if not is_dataclass_like(theclass):
raise ValueError(f'Expected "theclass" argument to be a dataclass-like, given {theclass}')

doc_group = get_doc_short_description(theclass, logger=self.logger)
for key in ["help", "title"]:
if key in kwargs and kwargs[key] is not None:
doc_group = kwargs.pop(key)
group = self._create_group_if_requested(theclass, nested_key, as_group, doc_group, config_load_type=theclass)

defaults = {}
if default is not None:
if isinstance(default, dict):
with suppress(TypeError):
default = theclass(**default)
if not isinstance(default, get_unaliased_type(theclass)):
raise ValueError(
f'Expected "default" argument to be an instance of "{theclass.__name__}" '
f"or its kwargs dict, given {default}"
)
defaults = dataclass_to_dict(default)

added_args: List[str] = []
param_kwargs = {k: v for k, v in kwargs.items() if k == "sub_configs"}
for param in get_signature_parameters(theclass, None, logger=self.logger):
self._add_signature_parameter(
group,
nested_key,
param,
added_args,
fail_untyped=fail_untyped,
default=defaults.get(param.name, inspect_empty),
**param_kwargs,
)

return added_args

def add_subclass_arguments(
self,
baseclass: Union[Type, Tuple[Type, ...]],
Expand Down Expand Up @@ -618,8 +575,6 @@ def dataclass_to_dict(value) -> dict:
pydantic_model = is_pydantic_model(type(value))
if pydantic_model:
return value.dict() if pydantic_model == 1 else value.model_dump()
if isinstance(value, LazyInitBaseClass):
return value.lazy_get_init_data().as_dict()
return dataclasses.asdict(value)


Expand Down
2 changes: 0 additions & 2 deletions jsonargparse/_typehints.py
Original file line number Diff line number Diff line change
Expand Up @@ -1592,8 +1592,6 @@ def lazy_get_init_args(self) -> Namespace:

def lazy_get_init_data(self):
init_args = self.lazy_get_init_args()
if is_dataclass_like(self._lazy_class_type):
return init_args
init = Namespace(class_path=get_import_path(self._lazy_class_type))
if len(self._lazy_kwargs) > 0:
init["init_args"] = init_args
Expand Down
Loading

0 comments on commit e9eca4c

Please sign in to comment.